fsharp / fslang-suggestions

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

Support `and!` for task based CE #1363

Open Lanayx opened 3 months ago

Lanayx commented 3 months ago

I propose we add support and! for task based CE

The use case is the following - we need to load several pieces of data that are not interdependent to combine them and use further on in program. Rather than doing those calls sequentially one after another it is beneficial to perform those calls concurrently and continue when all the pieces are loaded - performance optimization. Actually, this is even covered in documentation although not implemented https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/computation-expressions#and

So basically it would be possible to simplify this code

task {
    let tOne= getData1()
    let tTwo= getData2()
    let! results = Task.WhenAll([| tOne; tTwo|])
    let one = tOne.Result
    let two = tOne.Result
    ...
}

into this code

task {
    let! one = getData1()
    and! two = getData2()
    ...
}

More examples

The existing way of approaching this problem in F# is 1) the first snippet from above 2) starting all tasks first and them awaiting them one by one

task {
    let tOne = getData1()
    let tTwo = getData2()
    let! one = tOne
    let! two = tTwo
    ...
}

Pros and Cons

The advantages of making this adjustment to F# are clear and concise way to concurrently await multiple tasks

The disadvantages of making this adjustment to F# are i don't see disadvantages

Extra information

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

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick these items 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.

brianrourkeboll commented 3 months ago

More discussion (for async, but similar ideas apply):

abelbraaksma commented 3 months ago

Why not just write:

task {
    let! one = getData1()
    let! two = getData2()
    ...
}

There's no need to first assign a task to a variable and then let-bang bind it. You can do that in one go.

clear and concise way to deal with launching parallel tasks

Maybe start with use-case like that? It's not quite clear from your main description, right now ;). But before you update anything, iirc, this has already been decided a while ago. Let me see if I can find the original suggestion.

EDIT, I see that @brianrourkeboll beat me to it. Specifically, see this comment: https://github.com/dotnet/fsharp/discussions/11043#discussioncomment-345815

Lanayx commented 3 months ago

@brianrourkeboll I specifically didn't mentioned async, since it's much harder problem to deal with and and! doesn't fit very natural there, while for task it does

Lanayx commented 3 months ago

@abelbraaksma The use case is very common - to collect data from several sources before doing some business logic with the combined object, while those calls are not dependent one on another. So basically a performance optimization, no need to wait for callbacks sequentially

abelbraaksma commented 3 months ago

Your example originally hot-started the tasks before the let-bang. After doing that, they cannot be parallelized anymore, depending how they were started (they may have started a child task, though, but that's not under your control). With Async (always cold-started) scheduling parallel tasks is a breeze, but with tasks (as used in F# through task {...}) it is not, because of them already potentially been started.

Not saying that it cannot be done, but there are a whole bunch of intricacies involved, and that's on top of the ones for the (far more natural) async (but that one is available in IcedTasks, it's a cool lib!).

abelbraaksma commented 3 months ago

The use case is very common - to collect data from several sources before doing some business logic with the combined object,

Yes, it is indeed a common use case. I was merely saying that it was not clear from your example (there's nothing in there that's parallel and the text doesn't mention it). Using tasks without awaiting them is (probably) not what you are after. And hot tasks are not meant for parallelization (they are already started, after all).

The common use case has been recognized in many ways, and recently (.NET 9) has added a parallel for-each, for instance. I'm not saying we shouldn't improve (I'd love to have more parallelism support), but I don't think this approach is feasible given the current design of task (but I'm happy to be proven wrong though).

Lanayx commented 3 months ago

@abelbraaksma Thanks for clarifying that, I've updated the description, probably the word parallel made a confusion

abelbraaksma commented 3 months ago

Tx, but actually, reading "parallel" was the first time I understood what you were after (or maybe I still misunderstand, not sure now). Your example doesn't show a use-case (it's just not something you should do). Ideally, your code (and the description and intro of that code) would explain your use-case. There's nothing in your current code that requires and to be needed.

The better that example shows what you want (and why it doesn't currently work), the more people can upvote it. They won't upvote what they won't get.

Lanayx commented 3 months ago

Updated the description again, hopefully it makes sense now :)

abelbraaksma commented 3 months ago

Much better! ❤️

Lanayx commented 3 months ago

Custom extension will work, however built-in implementation is beneficial:

type TaskBuilder with
    member this.MergeSources(task1: Task<'a>, task2: Task<'b>) =
        task {
            let! _ = Task.WhenAll(task1, task2)
            return (task1.Result, task2.Result)
        }
abelbraaksma commented 3 months ago

yeah, that'll work a little bit. But keep in mind that F# tasks are hot. That means that task2 (or task1) may already have finished before calling Task.WhenAll and they cannot be scheduled on a different thread anymore. This may or may not be what you want. It won't introduce parallelism. But if it's already there, it'll work as expected.

From your original description, I think you want Async instead, so you can actually control when and how they are run, and control that they are run in parallel.

Lanayx commented 3 months ago

@abelbraaksma Actually I didn't want to control where and how tasks run (only that they can be started independently), because tasks are hot and as user I only care about their completion, since I can safely assume they are already started. That's the reason why it's so natural to do this for tasks and not for asyncs.

abelbraaksma commented 3 months ago

That's the reason why it's so natural to do this for tasks and not for asyncs.

Maybe I am missing something here, or just totally misunderstood your suggestion. Once a task starts running, you don't want to force them to run in parallel (which your original code does). But now you way that you do not want to control how tasks run, except that that's precisely what your proposed code is doing?

Sorry if I misunderstood. I'm just really confused. Either you await tasks, or you don't, but you (typically) do not want to have them run together, unless you know that parallel running is safe, and that really depends on what your code is doing.

only that they can be started independently

I don't understand this line. They are started when they are encountered by the JIT. This is why, for instance, a List<task> is such a bad idea (each task would start immediately, running concurrently), and why, in the general case, you do not want to have let myFunc (t1: task<_>) (t2: task<_>), unless you do this for delayed tasks (again, because they would run concurrently).

Do you, by any chance, just mean to use Array.Parallel or Async.Parallel? These are meant to do what you appear to describe.

Perhaps, for illustration, could you show a practical use-case? It may help me (or others) help understand what is suggested here. I don't want to dismiss a good suggestion on the grounds of me not understanding it ;).

Lanayx commented 3 months ago

@abelbraaksma sorry to confuse you again, this feels so simple to me that I can't explain it in simple words. Maybe the code will speak clearer Case1:

open System.IO
let joinTwoFiles fileName1 fileName2 =
    task {
      let! content1 = File.ReadAllLinesAsync fileName1
      and! content2 = File.ReadAllLinesAsync fileName2
      return content1 + content2
    }

Case2:

let getBestPrice provider1 provider2 =
    task {
      let! prices1 = httpClient.GetFromJsonAsync<Price>(provider1)
      and! prices2 = httpClient.GetFromJsonAsync<Price>(provider2)
      return Math.Max(prices1.Value, prices2.Value)
    }

Case3:

let loadFullConfiguration getDbConfig getFileConfig =
    task {
      let! dbConfig = getDbConfig()
      and! fileConfig= getFileConfig()
      return { DbConfig = dbConfig; FileConfig = fileConfig }
    }

As you see, I'm not specifying how or where to start tasks, just calling methods returning them and awaiting the results independently (concurrently)

abelbraaksma commented 3 months ago

Thanks. It does make your intent clearer. Imo, this should be governed by a user extension, as this is rather scary to open up. Besides, there are other suggestions with and! and tasks/async that appear to want to use it for parallelism. It can get really confusing really quickly.

But I do see the merit in the applicative nature of this proposal. It is more of a natural fit than parallelism, I guess. But people might expect this to run in parallel, or out of order, potentially breaking their app (which is why I think a user extension is the better solution: each project has its own specific use-cases for this).

But I may be completely wrong about where this should land, of course. Let others please chime in on what their ideas are, after all, this is community driven :).

PS: Just a tip: I'd advise you to update your original post regularly. People won't scroll down to read everything. Specifically your last post really shows what you actually want and may lead more people to vote for this: just replace your current examples with these for clarity.

TheAngryByrd commented 3 months ago

FWIW: IcedTasks does have this which you can use as a polyfill

voronoipotato commented 1 month ago

@abelbraaksma I think the ergonomics of having no default implementation for and! are really really awful. I would prefer a less good implementation that actually exists than not having access to one at all. This also goes for how result ce doesn't have an and! to gather up a list of errors. I'm not attached to a specific implementation, but it absolutely should exist. I don't think each project has specific use-cases for this and I think you're vastly overestimating the competency with building CE's of the average F# user.

Most consumers will not use and! it at all because it has no implementation. You can tell by going to any number of projects. In a some cases people will use @TheAngryByrd 's IcedTasks, because the implementation actually exists. I don't think people value IcedTasks enough precisely because the lack of a default implementation means people never have an opportunity to see what value it could provide.