Skip to content

Let SegmentedList Handle more than 2^31-1 elements - #1486

Merged
brianrob merged 18 commits into
microsoft:feature/large-gcdumpfrom
mpeyrotc:dev/mapeyrot/segmented_list_fix
Sep 22, 2021
Merged

Let SegmentedList Handle more than 2^31-1 elements#1486
brianrob merged 18 commits into
microsoft:feature/large-gcdumpfrom
mpeyrotc:dev/mapeyrot/segmented_list_fix

Conversation

@mpeyrotc

@mpeyrotc mpeyrotc commented Sep 8, 2021

Copy link
Copy Markdown
Member

Since we want to support collections that are larger than 2^31 - 1 elements, this PR changes all our indexing logic to use longs and makes sure that in interface methods from ICollection and IReadOnlyList that require us to continue using ints we throw when we pass their max limit.

Bumps [url-parse](https://github.com/unshiftio/url-parse) from 1.5.1 to 1.5.3.
- [Release notes](https://github.com/unshiftio/url-parse/releases)
- [Commits](unshiftio/url-parse@1.5.1...1.5.3)

---
updated-dependencies:
- dependency-name: url-parse
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
@mpeyrotc

mpeyrotc commented Sep 8, 2021

Copy link
Copy Markdown
Member Author

@brianrob @sharwell this is a prerequisite to changing segmented list to support more elements. It should not have any functionality change. Could I bother you with a review?

@sharwell

sharwell commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

Since we want to support collections that are larger than 231 - 1

To clarify, you are referring to the number of elements in the collection or just the number of bytes it takes to represent those elements?

@mpeyrotc

mpeyrotc commented Sep 8, 2021

Copy link
Copy Markdown
Member Author

I fixed the message, elements.

@sharwell

sharwell commented Sep 9, 2021

Copy link
Copy Markdown
Contributor

What about just making ToArray() and CopyTo() throw an exception if the collection contains more than the supported number of elements?

@mpeyrotc

mpeyrotc commented Sep 9, 2021

Copy link
Copy Markdown
Member Author

That is true, although we don't use that code at all in any of the places that use this data structure, so I am fine with either solution.

Although, we would also need to change the indexer, as well as count. That was the main reason I removed the interfaces.

@brianrob

Copy link
Copy Markdown
Member

Agreed with @sharwell, I would like to keep the implementation, but let's throw if the operation can't be run because the collection is too large.

@mpeyrotc
mpeyrotc force-pushed the dev/mapeyrot/segmented_list_fix branch 3 times, most recently from 034994c to 1242006 Compare September 15, 2021 19:46
@mpeyrotc mpeyrotc changed the title No longer support Collections Interfaces in SegmentedList Let SegmentedList Handle more than 2^31-1 elements Sep 15, 2021
Comment thread src/FastSerialization/SegmentedList.cs
Comment thread src/FastSerialization/SegmentedList.cs Outdated
Comment thread src/FastSerialization/SegmentedList.cs Outdated
@brianrob
brianrob changed the base branch from main to feature/large-gcdump September 17, 2021 02:48
@mpeyrotc
mpeyrotc force-pushed the dev/mapeyrot/segmented_list_fix branch from 4baa765 to 7dd8b43 Compare September 21, 2021 00:17

@brianrob brianrob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this into the feature branch to unblock progress. I will do further review there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants