fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

[WIP] Remove trampoline/hijack protection in async.fs and rely on tailcalls being taken

Open dsyme opened this issue 4 years ago • 7 comments

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.

dsyme avatar Jul 08 '21 23:07 dsyme

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)

dsyme avatar Jul 09 '21 15:07 dsyme

It's interesting that this passes now. It's possible we should use this on .net core. I'll do some perf measurements.

dsyme avatar Jul 14 '21 00:07 dsyme

@dsyme, do we need to keep this active?

KevinRansom avatar Nov 22 '21 21:11 KevinRansom

Yes, we should merge it at some point. I'm not sure what other validation to do though. I'll get back to it.

dsyme avatar Nov 22 '21 23:11 dsyme

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?

edgarfgp avatar Jun 20 '22 17:06 edgarfgp

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.

dsyme avatar Jul 13 '22 11:07 dsyme

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

NinoFloris avatar Jul 13 '22 11:07 NinoFloris