Minor perf improvement to List<T>.Add#9323
Conversation
Small improvement to List.Add's code quality. In a microbenchmark that adds lots of integers to a list (without needing to grow), this improves throughput by ~10%.
|
|
||
| // Separated out of List.Add to improve its code quality | ||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| private void GrowCapacityForAdd() |
There was a problem hiding this comment.
Caching size in a local makes sense, but I would not expect that factoring out EnsureCapacity(_size+1) to separate method to help anything. Is GrowCapacityForAdd really needed?
There was a problem hiding this comment.
On the machine I was using before, it actually did make a measurable difference, and it resulted in different asm (beyond the expected changes related to args). I'm now on a different machine, though, and I'm not able to repro the perf difference. I'll take another look at it over the weekend or on Monday.
There was a problem hiding this comment.
I tried this; but NoIining was getting ignored as function was coming up as [below ALWAYS_INLINE size] couldn't determine if it was new behaviour or always like it. Was also doing it before the StackCrawl change, where caller would get inlined regardless if it was extremely simple.
There was a problem hiding this comment.
where caller would get inlined regardless if it was extremely simple.
This sounds like a bug that we should fix if you still see it. NoInlining should always mean NoInlining, no exceptions.
|
I'm going to close this; I've tried on a few machines now, and in some cases the new version is faster, in others it's slower. |
5 participants
Small improvement to List.Add's code quality. In a microbenchmark that adds lots of integers to a list (without needing to grow), this improves throughput by ~10%.
cc: @jkotas