fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
345 stars 21 forks source link

Add MailboxProcessor.StartImmediate method #997

Closed xloggr closed 6 days ago

xloggr commented 3 years ago

An original discussion started in the dotnet/fsharp as issue #11370 and has a detailed proposal description and a number of comments.

Pros and Cons

An advantage of making this adjustment to the core library is giving library users freedom to choose in which threading context MailboxProcessor will be started, rather than limiting them to the Async.Start. It also makes MailboxProcessor slightly more efficient in some cases, e.g. when running synchronously, avoiding context switching overhead. The change is trivial, fully backward-compatible, and enables using MailboxProcessor in more contexts.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): XS

Please see it implemented in the PR 11417.

Affidavit (please submit!)

Please tick this by placing a cross in the box:

Please tick all that apply:

For Readers

If you would like to see this issue implemented, please click the :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.

dsyme commented 3 years ago

See comment from @xloggr here https://github.com/dotnet/fsharp/issues/11370#issuecomment-818712480

dsyme commented 3 years ago

Following on from that comment - Are there any use-cases that aren't covered by Async.StartImmediate and Async.RunSynchronously?

The use of a starter function just seems odd as we don't do that sort of thing anywhere else, and it's not particularly simple to use, and also exposes use to weirdness where whacky functions raise exceptions, or run the async twice etc.

xloggr commented 3 years ago

Great questions, please let me address them one by one:

Following on from that comment - Are there any use-cases that aren't covered by Async.StartImmediate and Async.RunSynchronously?

I cannot pinpoint particular use cases but I think introducing two mbox.StartImmediate and mbox.RunSynchronously will only extend this specific library limitation instead of removing it altogether.

The use of a starter function just seems odd as we don't do that sort of thing anywhere else

F# is a functional language, so passing around functions as parameters should be natural. The fact that is hasn't been done before, probably only means there was no need to do so. Even if it is uncommon to use in FSharp.Core, defining and passing async starters is used in other F# libraries. For example, if you follow this link you can see it implemented in the Elmish library. Are there any reasons against doing it?

... it's not particularly simple to use

In my opinion, a library should be both flexible and convenient to use. Indeed, calling StartWith() is an advanced use case, however it can be well disguised behind paremterless extension methods, as described in my other comment. Most people will probably use the extension methods, while StartWith() method can be used for customizations not foreseen by the library designers. For example, it can be used by authors of libraries built on top of the FSharp.Core.

... exposes use to weirdness where whacky functions raise exceptions, or run the async twice

Even in the current implementation nothing prevents a creative developer from making the same whacky staff in the computation running within a MailboxProcessor. Thus, parametrizing "async starter" doesn't give any additional powers to the library users.

dsyme commented 3 years ago

I cannot pinpoint particular use cases but I think introducing two mbox.StartImmediate and mbox.RunSynchronously will only extend this specific library limitation instead of removing it altogether.

I believe once you have StartImmediate you can build anything that starts asynchronously (by writing appropriate async code that does whatever it wants with the async continuation).

RunSynchronously is a little odd. It's like waiting for termination of the inner loop. It feels like it should be a separate primitive such as mbox.AwaitCompletion.

F# is a functional language, so passing around functions as parameters should be natural.

Well, just because F# supports function values nicely doesn't mean we splatter function parameters around everywhere in API design.

xloggr commented 3 years ago

I believe once you have StartImmediate you can build anything that starts asynchronously (by writing appropriate async code that does whatever it wants with the async continuation).

Great point! In particular, it enables making the whole loop execution synchronous by calling Async.RunSynchronously within the "body" computation. In this case, indeed, there's no need to implement mbox.RunSynchronously and the whole change will amount to adding StartImmediate method to the MailboxProcessor class. mbox.StartWith can be kept private or refactored into an internal function - to be reused by both mbox.Start and mbox.StartImmediate implementations.

If you strongly feel against introducing "async starter" as a parameter, it can be a way to go.

RunSynchronously is a little odd. It's like waiting for termination of the inner loop. It feels like it should be a separate primitive such as mbox.AwaitCompletion.

Implementing mbox.AwaitCompletion will indeed achieve the same result as passing RunSynchronously as an "async starter" parameter. However, if the implementation uses Async.Start - the context switch overhead will still be incurred. Introducing StartImmediate as suggested above, though, will enable achieving the same result with no overhead.

xloggr commented 3 years ago

The proposed change is implemented in the PR# 11515 and is ready for review.

As discussed above, the only publicly visible change in the MailboxProcessor type is an addition of StartImmediate methods (instance and static).

dsyme commented 3 years ago

instance and static

It looks like there is just one instance method, which is good, that's what we want

bmitc commented 1 year ago

I have recently come across a use case of wanting to run a MailboxProcessor on a single, unique thread for its lifetime. The use case is in using a MailboxProcessor to manage UI related things, such as windows in GLFW, that have strong constraints about what's allowed across threads or even forcing things to happen on the same single thread that other stuff happened on.

This is in theory possible to do manually using Microsoft.VisualStudio.Threading.SingleThreadedSynchronizationContext with a Thread and Async.SwitchToContext, but at the moment, I am having trouble getting this going. But that's another issue.

It would be great for MailboxProcessor to have a method that starts it on a single, unique thread. Maybe StartOnNewThread is a good enough name with documentation stating it will be a new unique thread that will not be switched during the lifetime of the MailboxProcessor?

Does this proposed feature support this? If I am understanding correctly, it does, but I haven't had time to look at the proposed code, so I'm not sure what the API interface looks like.

bmitc commented 2 weeks ago

I believe this can be closed. It was implemented and merged in https://github.com/dotnet/fsharp/pull/14931. Here is the method in the F# MailboxProcessor documentation.