Don't do null check on every loop when filling buffer. - #2871
Conversation
Buffer has an optimisation of using a null array to represent an empty state, that also plays well with it being a value type. When the buffer is filled from an enumerable, this possible null state is checked for on every loop. Restructure to avoid this redundancy.
|
Great, I'll close my PR. Thanks for the heads-up. |
|
I compared some methods with just those two fragements, and measurements varied a lot depending on the source enumerable, from around less than 1% worse to 20% better (but only one case was that extreme), with most a couple of percent better. If the source was something like an array or a list that could iterate quickly by moving an index against an array it actually did quite a bit worse, but that case wouldn't be hit here. |
|
@James-Ko I didn't see that PR. GMTA. The addition of |
|
@dotnet-bot test this please |
There was a problem hiding this comment.
I liked the previous version that just touched the below code (optimizing away the null check on each iteration) much more than I like this version with the gotos. It seems like we're adding complexity and code simply to avoid an additional null check / branch on each ToArray call, but there's already a whole bunch of casts and null checks and branches on these code paths. Does this really amount to a measurable win?
There was a problem hiding this comment.
You're completely correct. The only case where it saves anything but a single compare is in the case of a zero-item collection source, and there not only does it not save much, but that case has a much more straightforward way of dealing with it by just bailing out then, anyway, but even that isn't worth the effort.
83c5c62 to
6cf2b4c
Compare
|
Have reset on the moment of madness in those last two commits. |
|
@hackcraft Agreed, I didn't think the gotos should be necessary either. |
|
LGTM. Thanks. |
|
@VSadov, you ok with the change? |
|
LGTM |
Don't do null check on every loop when filling buffer.
…ll_loop Don't do null check on every loop when filling buffer. Commit migrated from dotnet/corefx@83a5b06
7 participants
Buffer has an optimisation of using a null array to represent an empty state, that
also plays well with it being a value type.
When the buffer is filled from an enumerable, this possible null state is checked
for on every loop.
Restructure to avoid this redundancy.