[WIP] Remove trampoline/hijack protection in async.fs and rely on tailcalls being taken
I'm mostly just curious what CI says
The async implementation historically included trampoline/hijack protection so that, after a certain number of operations running an async, the continuation would be hijacked and execution restarted in the trampoline installed at the base of the callstack
This was needed largely because tailcalls could not be guaranteed, especially in partial trust code for .NET Framework 2.0, when tailcalls were often disabled, or if tailcalls were not taken on implementations of .NET like Silverlight.
For .NET Core, we should not need this, as tailcalls are now much more reliable. So this is an experiment to remove the hijack protection altogether and see what passes and what doesn't.
The trampoline is also used when exceptions happen when running asyncs, so is not removed entirely.
This uncovered a case where the F# compiler is incorrectly deciding to not emit a tailcall
I will look into this and fix it and then see if this goes green (again, out of curiosity, and to check our tailcall codegen)
It's interesting that this passes now. It's possible we should use this on .net core. I'll do some perf measurements.
@dsyme, do we need to keep this active?
Yes, we should merge it at some point. I'm not sure what other validation to do though. I'll get back to it.
While learning F# and about tailcalls in this Article . I came across this PR . I was wondering is this going be merged and how can affect performance?
I was wondering is this going be merged and how can affect performance?
In short, we don't know the answers to these. Merging is probably unlikely without specific concrete known benefits beyond code simplification, because this makes us considerably more exposed to glitches in tailcall implementations.
The TPL does stack probing to see if it should restart on a fresh stack, couldn't we do that here?
https://github.com/dotnet/runtime/blob/d3af4921f36dba8dde35ade7dff59a3a192edddb/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs#L3401
It's a netstandard2.1 api but I can't imagine FSharp.Core being netstandard2.0 forever. https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.runtimehelpers.tryensuresufficientexecutionstack?view=net-6.0
Some of the BCL uses a wrapper class like StackGuard and others directly use the api, like the TPL.
https://github.com/dotnet/runtime/search?q=TryEnsureSufficientExecutionStack
https://github.com/dotnet/runtime/search?q=StackGuard