-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Proposal: pool-based list, similar to List<T>, but based on ArrayPool<T> #27023
Comments
I figured the best thing to do would be to spike it and measure the impact; so I whipped something together based on Here's the gist, and here's the results (wow this took a long time to run!). Conclusion: the only time it is even in doubt is for very small lists - pretty sure this could be optimized a little, too.
|
BTW:
In this proposal (binary) serialization has to be taken into account. So this would be a radical breaking change. |
@gfoidl there would be no field change in that event (it would still be |
@mgravell it is strange that you have such significant difference for The difference between I've run them (only for Core using your gist) on my localhost and results confirm my initial assumption. Can you check your results once again? Benchmark BenchmarkDotNet=v0.10.14, OS=Windows 10.0.16299.547 (1709/FallCreatorsUpdate/Redstone3) Job=Core Runtime=Core
|
For large arrays, it could be significant if it is causing it to GC, but: I can re-run later |
@OlegKarasik here's another run, just 100/500/1000 on CoreCLR this time (note: edited due to runtime snafu) BenchmarkDotNet=v0.11.0, OS=Windows 10.0.17134.165 (1803/April2018Update/Redstone4) Job=Core Runtime=Core
|
This is very strange. Initially when I saw you second run I thought that the difference can be caused by different versions of BenchmarkDotNet and different measurements mechanics (in the first run I used 0.10.14) but after upgrade I am still getting really strange results - There are also a few interesting points:
Benchmarks (100/500/1000 on CoreCLR with Presized: true, false) BenchmarkDotNet=v0.11.0, OS=Windows 10.0.16299.547 (1709/FallCreatorsUpdate/Redstone3)
Intel Core i7-7700 CPU 3.60GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
Frequency=3515624 Hz, Resolution=284.4445 ns, Timer=TSC
.NET Core SDK=2.1.302
[Host] : .NET Core 2.1.2 (CoreCLR 4.6.26628.05, CoreFX 4.6.26629.01), 64bit RyuJIT
Core : .NET Core 2.1.2 (CoreCLR 4.6.26628.05, CoreFX 4.6.26629.01), 64bit RyuJIT
Job=Core Runtime=Core
|
I agree that is unexpected. All I know is: what I get is what I get, and what you get is what you get. |
Probably from different amounts of RAM in the two machines; raised issue to add it to output dotnet/BenchmarkDotNet#846 The additional costs from the ArrayPoolList are from renting and returning the arrays; and in the List from GC; otherwise the results should be mostly comparable. |
quite possibly; I've got 64GiB here (I really should see about that 128GiB upgrade...), but SQL Server and about a dozen redis servers in WSL will have been chewing up a lot of that. Plus... you know, two instances of |
@mgravell @benaadams
This can be the reason. I have checked the source of Rent and Return and looks like they do lot of work with Thread Local Storage and maintains storage on per core basis. Do you know whether BenchmarkDotNet maintains Thread Affinity during the benchmarks? Can Thread migration be the reason? |
@OlegKarasik renting from the arraypool isn't faster than allocating; it reduces the pressure on the GC and has more predicable sustained performance than GCs and reduces the GC effects on the rest of the application that is allocating. Also adding a disposal check per @mgravell Feedback, public class MemoryMarshal
{
public Span<T> AsSpan<T>(this ArrayPoolList<T> list) {...}
} |
@benaadams I just never thought that the speed difference is significant. Thanks for pointing this out. |
@danmosemsft would it fit into your "PowerCollections" plan? |
@karelz Do you think there is a possibility for this feature request to get a higher priority? I do think something's got to happen sooner or later RE: collections pooling if not as part of the FW then 3-rd party. There is (at least) one community - based project already: https://github.com/jtmueller/Collections.Pooled that looks good but haven't seen any recent updates to its 2.0 "preview" release. Going to try pinging them there. |
@GrabYourPitchforks any thoughts about this idea? |
I see this as akin to the |
It doesn't look there's been any mention of this in this thread, but there is a library out there Collections.Pooled that looks to support a lot of scenarios related to this and posts some very impressive benchmarks. Might be worth looking into? We've started leveraging this library on our large calculation engine that ends up throwing a good amount on the LOH (arrays w/ multi-million counts of reference types) I'm 0% associated with the library in question, but thought I would throw it out there! |
@cmeyertons Collections.Pooled has been mentioned in this comment: #27023 (comment) The project seemed quiet when I last looked at it. Issues are opened and then closed without being answered. |
runtime/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs Lines 880 to 883 in 1f4d0db
Should
Perhaps it is best to use |
Area:
System.Buffers
Currently, POCOs/DTOs often default to
List<T>
; a good solid implementation, but not very pool friendly, despite the fact POCOs etc often have very predictable life cycles. Additionally, when lists need to resize (growth), arrays are currently just dropped on the floor.To address these issues, and to make for effective memory usage for many scenarios, consider:
behaviour:
List<T>
Dispose()
returns array to poolObjectDisposedException
It would presumably need a custom
GetEnumerator()
, which for best performance should talk directly to the array; so to reduce the risk of this accidentally causing access to a returned array, it might be considered to be aref struct
enumerator, such that it cannot be stored anywhere. TheIEnumerable<T>
enumerator (explicit interface implementation) would presumably go via the indexer (so: localforeach
is fast;IEnumerable<T>
is safe but slower).Open questions:
Order : IDisposable
, there's a simple operation to do that, presumably after nulling array the field in the list but before returning it to the pool, to prevent recursion problems; perhaps an extension method on<T>(this ArrayPoolList<T> list) where T : class, IDisposable
that calls back into aninternal
method on the list?)Example usage:
Controversial ultra radical version: make
List<T>
do this, or add a newList<T>
subclass that does this. Disadvantage: can't deploy as easily to older frameworks like net4*The text was updated successfully, but these errors were encountered: