-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Allow support for params IEnumerable<T> methods #36
Comments
This feature was already proposed for C# 6, but didn't make it due to scheduling. So my guess is that it's pretty likely it will make it into C# 7. |
If we are going to invest in a new Today low level APIs which expose a WriteLine(params object[] args)
WriteLine(object arg)
WriteLine(object arg1, object arg2)
...
// The above pattern lets the following bind without the object[] allocation
WriteLine("dog"); Switching to
An idea we've sketched around a few times is a struct of the following nature:
The |
@jaredpar It sounds like you want to use the I could see a signature like this working pretty well: WriteLine(params ArgIterator args) |
@sharwell yes and no. Those definitely achieve part of the problem: efficient way of calling a method with an arbitrary number of arguments. It doesn't hit the other part though which is easy interaction with .Net collections. |
I actually see your concern as a separate issue from |
Having yet another separate class to represent Also, the performance of using |
The intent of void Method(params Arguments<int> args) { ... }
void Example(IEnumerable<int> e)
{
Method(42); // binds to Method(new Arguments<int>(42))
Method(e); // binds to Method(new Arguments<int>(e);
} |
I'm still not seeing any advantage to having an |
The advantage is simply avoiding the allocation for the collection at the call site. It's not about specific optimizations within the method. The allocation may seem small, and typically is in isolated scenarios. Added up over an entire application though the array allocations add up (and quite fast). I've worked on several projects where we've measured significant performance improvements by adding a few overloads to This is why I don't see the value in adding
|
Which is a different algorithm by definition since you're treating the arguments differently. You're better off keeping the specialized overloads since bouncing through a struct intermediary will be slower, even if you reference the fields containing the values directly. If you have to instead bounce through an indexer it will be significantly slower, on par with the speed of just working with an array. The purpose of supporting public void Foo(params int[] args) {
Foo((IEnumerable<T>)args);
}
public void Foo(IEnumerable<int> args) {
// enumerate here
} For those cases where the performance is important and you can behave differently given a small number of arguments the compiler story is already quite good through overload resolution. I see no need to complicate that and force a single code path which couldn't be a single code path anyway. |
Completely disagree. We've tested out solutions in the past and found that the allocation benefit dominates any sort of indirection you would get from the struct. Note:
The Why push for a solution that is slower and allocates more memory over one which is solves the same problem + additional scenarios? |
Because those additional scenarios don't benefit without adding onus onto the developer of the method to write even more code than they need to today. The following is slower than void Foo(params Arguments<int> args) {
for (int i = 0; i < args.Count; i++) {
int arg = args[i];
// do stuff here
}
} And the following is slower than overloads: void Foo(params Arguments<int> args) {
if (args.Count == 2) {
// assuming accessible public readonly fields, using indexers here is significantly slower
int x = args.arg1;
int y = args.arg2;
}
// need to handle other possibilities here as well
} The one place where this could be nominally faster is in the case of |
@HaloFour the edit typo |
@HaloFour talk about the bad time for typos. I meant to say the exact opposite of that :( Editted the comment. |
I agree, and in those cases I do think that it would be worthwhile for the C# team to emit specialized implementations of |
Hmm, and if you want more than 2 arguments what happens? Will the compiler go and create an array and pass it to And I can't help not to notice that the allocation cost problem comes up quite often and every time we end up with solutions that are either incomplete or have some other not so great effects. Back in .NET 2.0 generic collections got struct enumerators, great, allocations avoided. And then you look at the JIT generated code and go hrmm. It looks like RyuJIT will produce better code but it took "only" 10 years. Maybe at some point we need to accept that the system works the way it works and it has certain performance characteristics. If you want to make it better, well, use .NET Native, add escape analysis and stack allocation to it and call it a day. Just my 2 cents. |
Yes.
Correct but this is not a new problem. It already happens today with It is definitely something I would love to see solved. So far though an elegant solution hasn't presented itself. If it did though I would likely be very interested in that as well.
Struct based enumerators have been around since 1.0. It was the original way to have type safe, non-allocation enumeration. I do share the frustrations on enumerators though and I've written some thoughts about it here. http://blog.paranoidcoding.com/2014/08/19/rethinking-enumerable.html
Speaking as someone who's worked on a lot of perf sensitive applications over the years: allocations matter much more than most developers give them credit for. Most performance investigations end up doing little more than trying to reduce the GC time which translates into curbing unnecessary allocations. Any time we create a feature in the language that has unnecessary allocations, it's a feature that will likely be avoided by perf sensitive applications. I'd much rather focus on features that are applicable to all types of programs. |
I do like elements of your rethinking on In the end, though, I think I'd rather be stuck with one slightly-less-perfect method than have a bunch of disparate but similar methods. |
@HaloFour , But at the same point, unless you are writing some very high specialized code like LINQ where you have optimized paths for different type of @mikedn, But boxing is always the problem if you have incompatible types. If you need different types and presumably need different handling for said type, you have no choice but to box. As for the rest of your points I'm not sure. I like that @jaredpar 's idea as it help solves the extra overloads problem that my original suggestion was getting at.
Under Jared's model this would be one method with one implementation. I also agree that IDisposable is very useful on enumerators. Especially those generated from |
@mburbea Just supporting |
@jaredpar Actually struct enumerators were added in 2.0, check ArrayList for example, it has a class enumerator and its GetEnumerator returns IEnumerator. And yes, I read your blogpost and I know you worked with Joe Duffy on a certain project 😄. Oh well, I suppose it makes sense for the compiler and framework to strive to minimize allocations. The main problem is how will the indexer deal with IEnumerable. I suppose you'll end up with something like this, otherwise you'll end up enumerating multiple times: T this[int index] {
if (count > 2) {
var list = _enumerable as IList<T>;
if (list == null) {
list = _enumerable.ToArray();
_enumerable = _list;
}
return list[index];
}
... As for boxing, that's likely unavoidable. You'd need something similar to C++'s variadic templates to get make it work. |
@mikedn guess you learn something new every day. I would have sworn struct enumerators were in 1.0. :) |
@HaloFour , when I'm at the point where I'm taking variadic arguments I rarely will be doing much differently then with one argument or N arguments.e.g. I probably would use it like |
The JIT may place a struct in a register if it contains only one field that fits in a register (a field of type |
@mburbea In which case you'd likely not be writing those additional overloads anyway as they would serve no purpose for you and having an intermediate struct would be of no benefit, either syntactically or performance-wise. I believe where you would see the improvement of an intermediate |
@HaloFour What do you mean by the former can be more optimized than the default array enumeration? Array enumeration is as fast as it gets, there's nothing more optimized than that. Maybe you meant the opposite or I did I read it wrong? |
I think he means array enumerators aren't the fastest. e.g.
will actually get compiled as a for loop to avoid creating an enumerator. If you instead change the signature to |
I would like to: public void Foo1(params T[] args) { ... }
public void Foo2(params IEnumerable<T> args) { ... }
public void Foo3(params IReadOnlyCollection<T> args) { ... }
public void Foo4(params IReadOnlyList<T> args) { ... } Method calls: FooX(a, b, c); is translated as: FooX(new T[] { a, b, c }); |
Might as well include |
@HaloFour public void Foo4(params IList<T> args) { args.Add(t); ... } |
That would be a runtime error, just like it would be today if you passed an array manually. |
@HaloFour |
I agree with @SergeyZhurikhin. A method getting an |
Any such method is already taking that risk if they don't first check the This would also provide an option to pass an interface of an indexable list with Not that I care all that much either way. |
That's already perfectly legal with public void Foo4(params int[] args) {
args[1] = 5;
}
...
var d = new [] { 1, 2, 3 };
Foo4(d);
Debug.Assert(d[1] == 5); |
@HaloFour |
And this is one of my least-favorite feature in BCL. Standad collection hierarchy is so ugly, and it seems that it will be as it is forever, becuase backward compatibility is the holy cow for Microsoft. It's bizzare, that array implements IList, and half of methods throws NotAllowedException. Is it statically typed language or not? |
@Pzixel compatibility is actually a really important feature but that's another topic. The good news is that we as developers have access to modern, typesafe and well specified interfaces such as |
@aluanhaddad problem with IEnumerable us that it can be a query, which implies multiple consequences.
Proper collection hierarchy was posed several years ago on SO:
Becuase now if I want to create my own collection with Count property, I should provide multiple methods which I really do not need, I just want to tell to user IMHO BCL team should provide new hierarchy of interfaces, well elaborated, and insert it in current hierarchy. It won't break anything, we'l just have new interfaces (for example like it's showed above). So we'l have compatibility with much better hierarchy. But it seems that BCL team don't think that current situation is bad in any sense. But it obviously is, because of properties |
Just asking, is this would support generic params? I mean something like this void Iter<E,T>(params E collections) where E : IEnumerable<T>
{
// iterate T
} |
@Thaina you can replace IEnumerable with T[] and see if it works. I'm sure that it doesn't. |
@Pzixel so you just want an interface with a |
@aluanhaddad hmm, we have such interface, it's a |
@jaredpar I wanted to weigh in on your Arguments suggestion: Is there any reason why you only have one Arguments struct? -- It seems like the intelligent indexer you suggest could be as much of a bottle neck as the problem you're trying to solve. I suggest making several versions of the struct, and letting the compiler chose which one makes the most sense at compile time -- excuse the poor naming, but an example of the compiler decision could be:
(Supporting the last one is important, because for many of us, the whole point of this request is to avoid having two method signatures for params vs BCL collections.) This will allow a smaller footprint too (the struct only stores the exact data that it needs, and only performs the exact logic it needs to in the intelligent indexers, etc.). You could make an interface for the structs -- something like IArguments -- that way we could just have our signatures as "params IArguments". (If IArguments won't work because of boxing or some such, consider using function pointers to hook up the "intelligent" logic in the struct to emulate OOP behavior). Personally, I would prefer that our method signatures be allowed to use IEnumerable if we wish, for methods that don't require the use of indexers (the same exact compiler logic would work, because [I?]Arguments inherits IEnumerable) -- and it will allow us to mark methods as being able to accept object streams (i.e. long running "yield return" jobs, etc.). |
The point of the single |
@HaloFour I believe I addressed the boxing issue in my comment. |
Care expanding that into something that applies to the CLR? IIRC, the only way you could do this would be to force the method to be generic where the |
@HaloFour sure; it's a pretty common thing though (in non-OOP languages -- the idea is you at least get to avoid the speed/complexity hit of determining which block of code to run at runtime). Here's some pseduo code in the context of the original struct:
|
Adding delegates to the mix would only add to the overhead since delegate invocation is not cheap. That still doesn't solve the issue of what you'd be passing to this method. At best you'd pass the two delegates and incur the penalty of a n+1 delegate invocations. But that would mean that you're no longer passing a single Given that the entire point of |
We are now taking language feature discussion on https://github.com/dotnet/csharplang for C# specific issues, https://github.com/dotnet/vblang for VB-specific features, and https://github.com/dotnet/csharplang for features that affect both languages. This request corresponds to dotnet/csharplang#179 |
Currently, C# only supports
params T[]
, however it is rare that I actually need an array for my method. More often than not, I merely want iteration or to perform some sort of LINQ operators against the parameter. In fact, it is common for libraries to declare a version of the method that takes an IEnumerable and a T[], and have the T[] version call the IEnumerable variant.The implementation detail could still be that an array is made at the call site and sent to the method, the benefit is to simply avoid having to create the extra overload.
The text was updated successfully, but these errors were encountered: