Skip to content

Implement TakeLast performance optimizations and benchmarks #207

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Akeit0
Copy link
Contributor

@Akeit0 Akeit0 commented Jul 18, 2025

#204
Simple TakeLast(100) Benchmark.
I wonder if optimization by TryGetSpan is needed.
(In fact, it might have been better if ValueEnumerable had an InnerSkip functionality)

Method N Mean Error StdDev Gen0 Allocated
FromArray_Linq 1000000 228.8 ns 33.15 ns 1.82 ns 0.0086 136 B
FromArray_ZLinq 1000000 3,547,090.8 ns 171,427.16 ns 9,396.50 ns - 48 B
FromArray_ZLinq(PR) 1000000 469,546.8 ns 162,486.33 ns 8,906.42 ns - -
FromList_Linq 1000000 242.9 ns 28.69 ns 1.57 ns 0.0086 136 B
FromList_ZLinq 1000000 3,547,946.9 ns 122,781.05 ns 6,730.04 ns - 48 B
FromList_ZLinq(PR) 1000000 545,908.3 ns 4,946.87 ns 271.15 ns - -
FromEnumerable_Linq 1000000 4,456,278.1 ns 76,890.24 ns 4,214.61 ns - 1368 B
FromEnumerable_ZLinq 1000000 4,021,346.6 ns 164,733.85 ns 9,029.62 ns - 136 B
FromEnumerable_ZLinq(PR) 1000000 3,410,969.7 ns 1,775,177.79 ns 97,303.49 ns - 128 B

@Akeit0
Copy link
Contributor Author

Akeit0 commented Jul 18, 2025

Hmm... How should the test be passed?
BinaryOperations_SourceEnumeratorsDisposed
System.OutOfMemoryException : Array dimensions exceeded supported range.
I don't think this is a good way.

var totalCount = 0X7FFFFFC7;//Array.MaxLength
if (source.TryGetNonEnumeratedCount(out totalCount) ||0X7FFFFFC7 < takeCount)

@neuecc
Copy link
Member

neuecc commented Jul 18, 2025

This isn't about errors, but I'm generally avoiding having enumerators hold pools directly as fields (for example, OrderBy and Reverse deliberately wrap them https://github.com/Cysharp/ZLinq/blob/9218fc6c/src/ZLinq/Internal/RentedArrayBox.cs#L5-L6)
ValueRingBuffer might be going against this policy.

@Akeit0
Copy link
Contributor Author

Akeit0 commented Jul 18, 2025

I fixed the error and make RingBuffer class.

The test error happening on ZLinq.Tests.Linq.OrderBySkipTakeTest.OrderByTake is very strange. It has nothing to do with this PR and passes in my local environment.

edit
Since I missed array clear?

@Akeit0
Copy link
Contributor Author

Akeit0 commented Jul 21, 2025

I think this PR is ready to merge.

@Akeit0
Copy link
Contributor Author

Akeit0 commented Jul 21, 2025

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