-
Notifications
You must be signed in to change notification settings - Fork 287
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
Implementation of WaitUntil.CoroutineEnds #821
Conversation
Tests included
Scanned through the changes a bit. Looks pretty good already but I think we can generalize it a bit more so we can use this for more than just a coroutineends feature. Also I want to measure the performance of this again. |
As expected some allocations are happening. Seems to be about 144 bytes per call which does impact performance quite a bit.
Its important to note that this only happens when you use the delegate. There is also a performance hit without because the struct is now bigger but its much smaller. |
Not yet satisfied with the performance as there is alot of allocations going on (closures, boxing) but I don't think there's a way around this unless C# adds something like a In that regard writing this does exactly the same: foreach(var step in SubCoroutine()) {
yield return step;
} This should prevent most of the allocations (there is still a enumerator being allocated) and makes the WaitUntil struct smaller. It is however more verbose and less readable. To give some numbers to show how big this difference is:
Thats already a big difference, especially in allocations. But thats not all because in both benchmarks the WaitUntil struct is bigger due to the added field for the delegate. If we remove it the numbers change into this:
So this is gonna be a question what we will value more. The readability of With deeper nesting the differences will only get bigger so really doubting if we should add this utility method. |
If this is how it is, then I believe it's better to just remove the method altogether.. there are many ways to deal with it in case. I'll just close it and add a note in the docs that this there is the possibility to integrate a Coroutine inside another |
This is an implementation of
WaitUntil.CoroutineEnds
as described here.