Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support and! for task based CE #1363

Open
6 tasks done
Lanayx opened this issue May 21, 2024 · 17 comments
Open
6 tasks done

Support and! for task based CE #1363

Lanayx opened this issue May 21, 2024 · 17 comments

Comments

@Lanayx
Copy link

Lanayx commented May 21, 2024

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:

  • This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
  • This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

For Readers

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

@abelbraaksma
Copy link
Member

abelbraaksma commented May 21, 2024

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: dotnet/fsharp#11043 (comment)

@Lanayx
Copy link
Author

Lanayx commented May 21, 2024

@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
Copy link
Author

Lanayx commented May 21, 2024

@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
Copy link
Member

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
Copy link
Member

abelbraaksma commented May 21, 2024

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
Copy link
Author

Lanayx commented May 21, 2024

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

@abelbraaksma
Copy link
Member

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
Copy link
Author

Lanayx commented May 21, 2024

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

@abelbraaksma
Copy link
Member

Much better! ❤️

@Lanayx
Copy link
Author

Lanayx commented May 26, 2024

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
Copy link
Member

abelbraaksma commented May 26, 2024

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
Copy link
Author

Lanayx commented May 26, 2024

@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
Copy link
Member

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
Copy link
Author

Lanayx commented May 27, 2024

@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
Copy link
Member

abelbraaksma commented Jun 1, 2024

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.

@abelbraaksma abelbraaksma changed the title Support and! for task based CE Support and! for task based CE Jun 6, 2024
@TheAngryByrd
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants