Does System.Linq.Enumerable.Iterator<T> still need _threadId and .Clone()?

[neon-sunset]

When looking at codegen emitted for LINQ iterator types, it appears that protected Iterator() .ctor and abstract Clone() add overhead (hurts much more on NativeAOT, where in JIT DynamicPGO helps out quite a bit).

Given the existing discussion on e.g. potentially eliding version checks in List and similar patterns in Release builds, I would like to ask if streamlining LINQ internals would be on the table too, since the expectation for IEnumerabe<T>-returned types to be not thread-safe.

[danmoseley]

Linking issue you alluded to

#81523

[stephentoub]

I would like to ask if streamlining LINQ internals would be on the table too, since the expectation for IEnumerabe-returned types to be not thread-safe.

Streamlining in general, sure. Removing this check, I don't think so. It could alternatively be an Interlocked.Exchange, but if memory serves it was explicitly changed away from that for perf reasons. While it may not be documented to be thread-safe, I expect too much code depends on it being so to warrant changing it, especially since if you're using the iterator types that derive from Iterator, you're already leaving some perf on the table vs just open-coding loops.

What is the "overhead" you're referring to, other than the two calls to Environment.GetCurrentManagedThreadId?

[neon-sunset]

Aside from enumerator state having to be commited back to heap (Does switching enumerators to boxed structs and letting the compiler devirt-unbox them (which it can't do if I understand it right) make sense?), it's just the large amount of code for opportunistic cloning and building the state of iterator throughout its inheritance tree, which, depending on other contents of the method body, causes the loops over LINQ to be much slower than expected because CQ hits a wall after exceeding inlining/time budget.

To be clear, Dynamic PGO helps a lot, especially in benchmarks. For Native AOT, I'll need to re-check whether and how well exact devirtualization kicks in after relatively recent updates.

Also, I did not realize changes like #88249 would be accepted (due to affecting type tests in user code), thank you for this PR, especially since it addresses AddRange over arrays/lists. UPD: Select was reverted, unfortunate but not unexpected.

[tfenise]

abstract Clone() is not just needed to be thread-safe, it is needed whenever Iterator<T>.GetEnumerator() is called more than once, which may be uncommon, but still a perfectly reasonable and supported situation.