add StartImmediate method to the MailboxProcessor (#11370)

[xloggr]

No description provided.

[dsyme]

Please also add a test e.g. in tests\fsharp\core\controlMailbox\test.fsx

[dsyme]

I think remove this module, and inline each of its elements. They don't make code any clearer, thanks

[xloggr]

Done.

I adapted Start method unit test for the new StartImmediate method, although I'm not certain I did it correctly. For example, this line "c32398u6: MailboxProcessor null" - I just copied it. Should the referenced id (c32398u6) be unique for each test?
I see that the new unit test fails on Windows CI machine because it doesn't see the new StartImmediate method, but it succeeds in other environments including my local Windows machine. I am not very familiar with the FSharp.Core test organization and appreciate help with this issue.

[dsyme]

Yes, we need more testing, please add something that checks that a StartImmediate really happened

[dsyme]

Thanks for the updates, the code looks good.

We need more testing, as noted

[dsyme]

We need more testing than this - create a MailboxProcessor that does a side effect during it's start?

[psfinaki]

Hey @xloggr, would you like to continue on that?
Alternatively, if @dsyme and/or @vzarytovskii still consider this useful, I can take over.

[dsyme]

@psfinaki Yes, please feel free to take over this work. It's a fairly simple addition but useful

[xloggr]

Hey @xloggr, would you like to continue on that? Alternatively, if @dsyme and/or @vzarytovskii still consider this useful, I can take over.

Hi @psfinaki,

You are welcome to take over.
I implemented the change in the PR but has never been able to reproduce the issue manifested in the CI build - my local build always succeeded.
Good luck!

[psfinaki]

Alright, I hope to get to this during this week.

[bmitc]

I would love to see this implemented and have a direct use case for it right now. So if anyone wants any help, I am happy to help out where I can.

I made a comment in the issue (fsharp/fslang-suggestions#997). As I understand it, having a StartImmediate method would allow you to pass a starter method wrapping this StartImmediate to a new thread to run the MailboxProcessor on. This allows you to start the MailboxProcessor either on the current thread or a specified thread. Is that correct?

[psfinaki]

Hi @bmitc :)

Thanks for chiming in! I think your assumption is correct. I don't have much expertise with this particular area of code, so if you have a direct use case and motivation - feel free to finish this one off, this is an excellent issue to "warm up" and I haven't started on this yet.

You can fix this branch or open another PR, whatever you wish.

[bmitc]

I went ahead and created my own fork and branch to work on this. However, I have a question to help clarify how all this works.

I recently asked this on Stack Overflow, and the same question applies here.

Inside the MailboxProcessor is the async:

async { try
            do! body x
        with exn ->
            errorEvent.Trigger exn }

It is the outer async that gets started via Async.StartImmediate, but what happens with the line do! body x?

1. Does do! run the asynchronous body x on the same thread that the outer thread was started on?
2. And I still have the same question about Async.StartImmediate. Does the thread the async is started on stay the same throughout the lifetime of the process?
   • The extension of this question is really (1), in that, what happens with let!, use!, do!, etc. for async processes that are started with Async.StartImmediate?

Async.StartImmediate's documentation is pretty vague, and I think needs updating in general. But I also couldn't find any information on this elsewhere, and I'm afraid the underlying implementation of async is currently over my head.

[bmitc]

I did an experiment that seems to imply that Async.StartImmediate does indeed propagate down to contained async processes. Is this documented anywhere and/or can anyone confirm?

// async.fsx
let outerAsync = async {
    printfn "Outer async thread: %A" System.Threading.Thread.CurrentThread.Name
    do! async {
        printfn "Inner async thread: %A" System.Threading.Thread.CurrentThread.Name
    }
}

System.Threading.Thread.CurrentThread.Name <- "main thread"
printfn "Main thread: %A" System.Threading.Thread.CurrentThread.Name

printfn "Async.StartImmediate"
Async.StartImmediate outerAsync

printfn "Async.Start"
Async.Start outerAsync

Then running this:

PS > dotnet fsi .\async.fsx
Main thread: "main thread"
Async.StartImmediate
Outer async thread: "main thread"
Inner async thread: "main thread"
Async.Start
Outer async thread: ".NET ThreadPool Worker"
Inner async thread: ".NET ThreadPool Worker"

[psfinaki]

@bmitc great job drilling into that, admittedly printing the thread names in code is currently probably a much easier way to understand what's going on than digging into the code or the docs. I probably wouldn't give a better answer than the guy on the StackOverflow already did, in that

To get an actual continuation, you need a real async operation, async { } itself is not async but lets you use async.

That said, it's definitely not where we want to be with our docs, so absolutely feel free to clarify this tutorial, either within or - even better - parallel to the changes in the MailboxProcessor. We, maintainers, can think that we write things clearly, but after all it's our customers for whom it should make sense :)

[bmitc]

Just a note that I have resumed working on this and hope to have a new PR up soon.

[bmitc]

@dsyme I'm working on this in another PR from my own branch, and am starting to add tests. Above, you requested that tests be added to tests\fsharp\core\controlMailbox\test.fsx.

What is the difference between that location and tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Control\MailboxProcessorType.fs (i.e., adding a test to the FSharp.Core.UnitTests project)?

I've never added a test to F# before, and so I don't know the difference between these two locations in terms of preference or if one is for certain types of tests or another reason. My initial thought would have been to add tests to FSharp.Core.UnitTests in the MailboxProcessorType.fs file.

[vzarytovskii]

I've never added a test to F# before, and so I don't know the difference between these two locations in terms of preference or if one is for certain types of tests or another reason. My initial thought would have been to add tests to FSharp.Core.UnitTests in the MailboxProcessorType.fs file.

We are trying to migrate away from the former test suite, so, plese don't add anything new there unless necessary, add tests either to FSharp.Core.UnitTests (which, are, well, unit-test style tests for the core library), or to the FSharp.Compiler.ComponentTests, which are testing whole compiler component, or both instead.

[bmitc]

Thank you @vzarytovskii! I will add to FSharp.Core.UnitTests. I was finding that easier anyway since I was having a bit of trouble understanding the test DSL in the .fsx file. Haha. There was one test I thought might be added there though, and that's calling StartImmediate twice since there is a test already there that does the same for Start. But other than that, I will add to FSharp.Core.UnitTests.

[psfinaki]

@bmitc thanks - sounds like a plan.

[0101]

@xloggr are you still planning to finish this?

[0101]

@bmitc are you still planning to finish this? :)

[bmitc]

I have been meaning to get back to this, as it stares me in the face in my PRs list. 😆 I believe where I left off was adding testing, as the change is not that complex. Let me get back it this week, as I think all I need to do is add some tests, and I can post a link to my PR here.

[bmitc]

I have updated my PR #14931 by updating my F# fork, rebasing on top of that, and adding a test, with some various other changes.

CI is passing, but I have left a self-review with some questions. I think it will be good to move discussion there.

[psfinaki]

Alright, I am closing this one then, thanks!