Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reworked GroupAdjacent and Batch to perform fully deferred execution #548

Closed
wants to merge 5 commits into from
Closed

Conversation

thibault-reigner
Copy link

@thibault-reigner thibault-reigner commented Nov 15, 2018

What's this PR do?

Simplify the code of GroupAdjacent and Batch to remove internal partial enumeration relying on instantiation of arrays and lists, hence performing fully deferred enumeration of the sources.
In my tests it improves the performances by avoiding allocation of large objects which often end up in the generation 2 part of the heap.

@leandromoh
Copy link
Collaborator

your Batch implementation return [3, 4, 5, 6, 7, 8, 9, 10, 11, 12,] for Enumerable.Range(1,30).Batch(10).ElementAt(2) while it must return [21, 22, 23, 24, 25, 26, 27, 28, 29, 30]. I already had this bug (and resolve it :D ) in my PR for a lazy Batch

@thibault-reigner
Copy link
Author

@leandromoh nice catch ! Indeed skipping the enumeration of a batch will generate incorrect results for the next one.

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.

2 participants