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

Prefer IEnumerable<object> over IEnumerable #640

Closed
Lingxi-Li opened this Issue Jul 18, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@Lingxi-Li
Contributor

Lingxi-Li commented Jul 18, 2016

IEnumerable is deprecated. IEnumerable<object> should be used instead. For example, see the Items property of ODataCollectionValue. This allows LINQ extension methods to be used, which are defined on IEnumerable<T>, not IEnumerable. As an example, see the Skip extension method.

@Lingxi-Li Lingxi-Li self-assigned this Jul 18, 2016

@Lingxi-Li Lingxi-Li added in-progress and removed task labels Jul 19, 2016

@Lingxi-Li

This comment has been minimized.

Contributor

Lingxi-Li commented Jul 19, 2016

Relates to this SO thread.

@Lingxi-Li

This comment has been minimized.

Contributor

Lingxi-Li commented Jul 19, 2016

Some practical implications of this change:

  1. Direct support for LINQ extension methods.
  2. You can no longer do = new[] { 1, 2, 3 }, but have to = new object[] { 1, 2, 3 } for value-typed element type.
  3. Better performance for value-typed element type, because boxing is performed only once at the time the enumerable is assigned. In existing code, boxing needs to be done every time the enumerable is read.
@Luaancz

This comment has been minimized.

Luaancz commented Jul 19, 2016

Have you done performance testing to confirm your suspicions?

In the old case, the array is a nice contiguous array with no indirection - very fast. Whatever boxing you need to do is done on a gen-0 heap and discarded almost immediately - very fast.

In the "improved" case, every single element of the array is a separate object for the whole lifetime of the array, and changing an element in the array means spreading the objects over memory, rather than keeping the array contiguous. And since you have an array of objects, every single item is an unknown - what if someone puts a string where you expect an int?

If you really want to keep the old system, use .Cast<object>. That gives you the same characteristics as the old approach (boxing on iteration, not in the array), and wherever you know the correct type at compile-time, no boxing is necessary (as with your sample code - just make the local int[] or IEnumerable<int>).

@Lingxi-Li

This comment has been minimized.

Contributor

Lingxi-Li commented Jul 19, 2016

In the old case, the array is a nice contiguous array with no indirection - very fast. Whatever boxing you need to do is done on a gen-0 heap and discarded almost immediately - very fast.

Nice point. I haven't thought of that. Will conduct a benchmark test to see whether this change does improve performance in our particular case (we have a standardized benchmark framework).

@Lingxi-Li

This comment has been minimized.

Contributor

Lingxi-Li commented Jul 20, 2016

Items to be fixed in V7:

  • Fix ODataCollectionValue.Items
@Lingxi-Li

This comment has been minimized.

Contributor

Lingxi-Li commented Jul 20, 2016

Current benchmark test suggests no meaningful performance difference before and after this change. So, remove the performance tag. Still push this change for its benefit of supporting LINQ extension methods directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment