4.x: Make plain Empty<T>() a singleton. #544
Conversation
| @@ -28,13 +29,27 @@ public _(IObserver<TResult> observer, IDisposable cancel) | |||
|
|
|||
| public IDisposable Run(IScheduler scheduler) | |||
| { | |||
| return scheduler.Schedule(Invoke); | |||
| return scheduler.Schedule(this, Invoke); | |||
There was a problem hiding this comment.
That will unfortunately due to this not save the allocation of a delegate, if you make it an anonymous lambda, it will.
There was a problem hiding this comment.
So I should make the body of Invoke a lambda here? I'll update the PR shortly.
Also a phylosophical question: why do I have to fight the optimizer so much?
There was a problem hiding this comment.
You mean, why doesn't Roslyn cache the delegate for a static method?
There was a problem hiding this comment.
Yes, Lambdas aren't new so I'd expect compiler optimizations already present for them.
There was a problem hiding this comment.
For lambdas, sure, there are and always were. But 'Invoke' is a method group, not a lambda. It's probably easy to implement but breaking.
There was a problem hiding this comment.
You may as well just leave it like that, Roslyn will eventually catch up I guess and then it's free lunch.
There was a problem hiding this comment.
Who knows how long 4.1 will stick with people. Let's have as many optimizations now without waiting for compiler support.
This PR changes the no-argument
Empty<T>operator into a singleton. There is practically no state to this operator and the current implementation schedules work on theImmediateScheduler, which is almost no-op (the delegate that signalsonCompletedgets executed immediately and there is a throwawayAsyncLockScheduleras well).The instantiation overhead inside the
Producer<TTarget>may be avoided as well but currently the type serves as the indicator for trusted source and safeguards are not placed. A plainIObservableimplementation fails 2 unit tests because the applied safeguards consume a thrown exception instead of letting it bubble up.Update:
I've also changed the scheduled version to use the
Schedule<TState>(TState, Func<...>)method to avoid allocating a delegate due to capture ofthis.