search

dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.88k stars 783 forks source link

Cancellable: Use AsyncLocal instead of ThreadStatic #17156

Closed majocha closed 4 months ago

majocha commented 4 months ago

Similar to #16779, Keeping the token in AsyncLocal could work better in scenarios with lots of parallelism like we have in Transparent Compiler.

todo: add some tests mixing cancellable with async, TPL, thread switching etc.

related: #16137, #16348, #16536

github-actions[bot] commented 4 months ago

:heavy_exclamation_mark: Release notes required

:white_check_mark: Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/8.0.400.md
majocha commented 4 months ago

@auduchinok, Does it make sense?

auduchinok commented 4 months ago

@auduchinok, Does it make sense?

From a glance, yes, thanks! I’ll probably need to do some testing just in case, though :)

majocha commented 4 months ago

I'm trying to write some tests using cancellable CE but I'm hitting FS1118, FS1113.

majocha commented 4 months ago

OK, the problem with testing the cancellable CE is as described here: https://github.com/dotnet/fsharp/issues/7110

It is internal and exposed to the test project via InternalsVisibleTo, which clashes with inlining.

As explained here:

InternalsVisibleTo doesn't allow inlining across assembly boundaries and if you try to use both of these you will get the above errors. So this is by design.

I can't think of a way around this limitation.

vzarytovskii commented 4 months ago

Yeah, I don't think there's a way around it. When I tried to rewrite it to resumable code, I hit some issues with inlining cross projects.

majocha commented 4 months ago

One thing that comes to my mind is to include link to Cancellable.fs in the test project itself. That could work if the code does not depend on some other internal inlined functions.