Run the selector on items during Select.Count chains.

[jamesqo]

Per @VSadov's comment at dotnet/corefx#12703 (comment):

Re: can we skip selectors/predicates in this optimization

It is a tough one.

Generally, assuming that anything can have sideeffects is indeed very
constraining to what kind of optimizations we can do. In some cases it
is hard to specify how much of the user code runs and in what order.
We definitely reserve the rights to substitute iteration for indexing
when we find that possible.

After thinking about this for quite a while, I think Count is a kind
of aggregating operator. Even though the actual results are not
collected, it seems reasonable for the user to expect that they would
be computed, and as such he may expect to observe the sideeffects from
selectors/predicates etc.

I.E. – I could see someone using a selector that writes to the
console, and use Count as a way to run the query for sideeffects only.

I think we should not take the change and check for other changes like
this, that we might have accepted in the past. (like: dotnet/corefx#11841 )

Here is what I think is the root cause - We do have a method for
obtaining counts internally -

GetCount(bool onlyIfCheap)

I think the method should not be used to bypass selectors when
actually running the query. It is ok to use it to preallocate internal
buffers. That still have an assumption that the “cheap” way of
obtaining the underlying “.Count” is idempotent, but it is a kind of
assumption that we agreed in the past to be acceptable.

It does not seem to be acceptable to assume that user-supplied
selectors/predicates are sideeffects-free in a context of aggregating
query.

I.E. –

it would be ok to preallocate a buffer based on GetCount(bool
onlyIfCheap) it would be ok to compute actual Count via , but only as
long as there are no funcs to run if we have selectors/predicates or
other funcs, they must run in aggregating queries.

So in other words, we should undo all the GetCount optimizations that skip running the selector if onlyIfCheap if false. But if it's true, we're probably using it to preallocate a buffer of some sort, & are probably going to use it anyways. So in those scenarios it should be ok to not run the selector.

cc @JonHanna

[JonHanna]

There's an irony here that the idea of a ForEach<T>(IEnumerable<T> source, Action<T>) was argued against (elsewhere, not here) on the basis that linq should really be dealing in pure functions, but that makes it all the more likely that there could be Count() called on something with delegates that aren't purely functional.

I think I reluctantly agree with @VSadov on this.

Unless you or someone else has added further uses of it during the summer when I wasn't paying much attention to corefx, I think Count() is the only place where onlyIfCheap starts as false, with all other uses either passing true or passing what it received upstream. So the silver lining is we can at least simplify it to only ever consider the onlyIfCheap case.

[VSadov]

The call is really whether it is possible for a customer to have code like

  // calling Count, just to make sure all elements are validated
  someEnumerable.Select(e=>Validate()).Count();

I think the code like above is quite possible.

Also it is a slightly different case from direct Count like in

  someEnumerableThatMightBeAList.Count();

In the first case we have a selector supplied by the user. I think he expects the selector to run.
In the second case he just wants to get the Count, and therefore we could check if we can know the count cheaply.

I.E. It feels ok to bypass an implied empty “foreach” when collection provides Count directly. It is reasonable to expect that foreach/GetEnumerator implementation is consistent with what Count does.
The selectors are, however, a kind of 3-rd party code plugins that would run inside that “foreach”, and I do not feel as brave about bypassing “foreach” when it contains 3rd party code.

[JonHanna]

I think that it may suffice to make the changes in fc53753adb142540d7fbd7766310cef4c959a65a (scratch code, untested) which reject the fastpath counting in those Select iterators that support it when the ultimate source is Count(), while still allowing it to be used for pre-allocation.

[jamesqo]

@JonHanna, are there other methods besides Select that need to run selectors in this method? e.g. OrderBy?

[JonHanna]

OrderBy always runs its key selector once per item no matter how much final selection is elided by the optimisations last year, because it can't compare until then.

Some things coming after a Select could skip items, but only if they decide they can pre-compute sizes (I'm not 100% about that, esp. since there's been a few changes since I was last at the point of being able to do the mental load-the-whole-project-into-RAM thing) so if we stomp on the GetCount() attempt to shortcut when it hits a selector I think we should be good. At least as far as people using Count() to force function invocation goes.