rspeele / taskbuilder.fs Goto Github PK

Hi all,

A question for people who use TaskBuilder, in relation to https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1097-task-builder.md

As everyone might know, the proposed support for tasks in F# is very much based around the functionality of TaskBuilder and we started with the test suite (though the implementation is entirely new).

Now TaskBuilder supports ContextInsensitive tasks where ConfigureAwait is false by default, e.g. https://github.com/rspeele/TaskBuilder.fs/blob/master/TaskBuilder.fs#L352. However the task { ... } support in the F# RFC doens't include ContextInsensitive tasks.

So the question is - how important is it for inbuilt F# task support to support a builder (e.g. open ContextInsensitive ... task { ...}) for context insensitive tasks? Or is it ok for people to do this manually?

Personally I think moving away from the UI thread should be done explicitly, e.g.

task {
    do! Task.SwitchToBackgroundThread() // or whatever

    ... } 

or via explicit ConfigureAwait calls.

Hi, first of all thanks for building this fancy task builder! From my first few tests it works like a charm! I came across it, because someone used your TaskBuilder.fs to improve the performance of the Giraffe web framework and therefore I would be interested in consuming it via a NuGet package rather than having to copy/paste it every time you improve it or bug fix something. It would also make it easier by having it as a well documented dependency and avoid other contributors to try and change the TaskBuilder.fs in Giraffe when really they should probably discuss any changes/improvements with you here first so that everyone can benefit from it.

Would you be interested in making your TaskBuilder.fs accessible via a NuGet library? If yes I can even help out with a PR myself. Let me know and thank you for open sourcing it!

Hi, do you plan or any idea to make a TaskResultBuilder which similar with AsyncChoiceBuilder.

type AsyncChoice<'T, 'Error> = Async<Choice<'T, 'Error>>
type AsyncChoiceBuilder () =
    member __.Return x = // 'T -> M<'T>
    member __.Bind m f = // M<'T> * ('T -> M<'U>) -> M<'U>

type TaskResult<'T, 'Error> = Task<Result<'T, 'Error>>

Hello,

Thanks for this useful library.

What is the guidance for working with non-generic Tasks? A simple repro of my issue:

let doesThisWork : System.Threading.Tasks.Task =
    task {
        do! System.Threading.Tasks.Task.Delay(1000)
    }

Which produces this error:

FS0001    This expression was expected to have type
    'Threading.Tasks.Task'    
but here has type
    'Threading.Tasks.Task<unit>'

One workaround we have is:

type System.Threading.Tasks.Task with

    static member Ignore (task: Task<'T>) : Task =
        task :> Task

And then:

let doesThisWork : System.Threading.Tasks.Task =
    task {
        do! System.Threading.Tasks.Task.Delay(1000)
    } |> Task.Ignore

But this feels rather hacky.

https://blogs.msdn.microsoft.com/pfxteam/2010/05/24/why-is-taskcontinuationsoptions-executesynchronously-opt-in/

I've also been working with getting F#'s TPL usage up to the perf level that C# has for a few of our ASP.NET Core middlewares and judiciously adding TaskContinuationOptions.ExecuteSynchronously for small function bodies like exception handlers etcetera really made a huge difference in that code's performance profile.

The hard part about this however is that there is no way of accessing these levers while working with a CE. As I see you have done some really nice work on writing a performant CE I hope you might have some good ideas we, the F# community, are able to pursue.

I think it's a pity the TPL is natively so badly supported as it's crucial in working with many of the libraries written in C# these days.

Functions.StartTransaction. Microsoft.Azure.WebJobs.Script: One or more errors occurred. (Could not load type 'BindI' from assembly 'TaskBuilder.fs, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.). StartTransaction: Could not load type 'BindI' from assembly 'TaskBuilder.fs, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.

I get this error message when using TaskBuilder.fs prerelease on azure functions.

/cc @gusty @dsyme

This is a follow up on #15 to focus on how we release PR #14 .

TaskBuilder 1.1.1 (I think) already fixed @forki 's errors, the ones that #15 was originally about. It is stable.

1.1.1 and 1.2.0-rc have the same features and performance characteristics. They use the same underlying code for bind, tryFinally, etc. The difference is that thanks to @gusty rewriting the builder classes, 1.2.0-rc should work with older versions of the F# compiler, and will also be compatible if future versions of the compiler revert the type inference changes that came with F# 4.1. See the F# compiler issue. I consider this change good but not urgent, so we have time to do this smoothly.

TaskBuilder 1.2.0-rc is "source compatible" with code written for earlier TaskBuilders, but not binary compatible. I.E. if you upgrade you shouldn't have to edit any of your code -- BUT you will have problems mixing the two .dlls in one solution. If you have one project referencing 1.1.1 and another project referencing 1.2.0-rc, you can't then reference both projects in your web app or whatever. I don't think binding redirects fix this.

I do agree that 1.2.0-rc should have been versioned as 2.0.0-rc. When I versioned it I was only thinking about the source compatibility not the binary compatibility.

I guess we could take route 1 or route 2. Either way, the next version I publish will be 2.0.0.

1. Publish 1.2.0-rc as 2.0.0. and have Giraffe coordinate with their downstream users so everybody updates TaskBuilder together. This keeps the code clean but will cause a headache or two downstream.

2. Revise 1.2.0-rc to include both builders, with the new @gusty ones in a v2 namespace. Mark the old builders as obsolete. This is messy but smooth as it would make everything binary compatible.

To elaborate on route 2, suppose you currently you reference TaskBuilder 1.1.1. Upon upgrading to 2.0.0, you would get warnings because you are using the obsolete builder in every file where you have a task {...} block. You would fix those warnings by changing open FSharp.Control.Tasks.ContextInsensitive to open FSharp.Control.TasksV2.ContextInsensitive. If you depend on a library that was written against 1.1.1, it will still work, because the builders in the old namespace still exist and there is no conflict between the two.

What's the preference of folks using this?

do! isn't happy with a Task<_> and let! _ = can be a work around, but not very readable.

FSharp Async as Async.Ignore. But I don't see an equivalent with this library, and it seems like there should be.

Since Task<_> inherits from Task, I'm thinking

let Ignore (resultTask:Task<_>): Task = upcast resultTask

should work. But we are in the middle of porting some C# to F# code, and haven't tested it other than type checking wise it appears to work.

I was wondering if this is an issue anyone has dealt with.

Thanks,

I am not sure if I am doing something wrong or if this is a bug, but I am getting the following error when trying to do a do! binding:

The type 'Threading.Tasks.Task' is not compatible with the type 'Threading.Tasks.Task<'a>'

This is a netcoreapp2.1 project, with the latest TaskBuilder.fs and built on the latest 2.1.400 SDK.

I also opened open FSharp.Control.Tasks.V2.ContextInsensitive.

Did I miss something?

Works:

let fooTask = FSharp.Control.Tasks.ContextSensitive.task

let someOp =
  fooTask {
    do! Task.Delay(4)
  }

Doesn't work:

let fooTask = FSharp.Control.Tasks.ContextInsensitive.task

let someOp =
  fooTask {
    do! Task.Delay(4)
  }

With the following errors:

error FS0001: The type 'Task' is not compatible with the type 'Task<'a>'
error FS0193: Type constraint mismatch. The type 'Task' is not compatible with type 'Task<unit>'

Basically the insensitive builder seem to not have the generic support for awaitable that's in the code

Hi,

Do you think we could add an Async overload bind to the TaskBuilder?

It would be mainly for convenience to save an Async.StartAsTask as seen here.

Related to: giraffe-fsharp/Giraffe#225

How I can set my own TaskScheduler?

I'm looking at an incorporation into FSHarp.Core, see dotnet/fsharp#6811

Please let me know what you think, please follow that thread and please look at the design carefully. At the moment it follows the same general pattern as TaskBuilder.fs and the implementation is based on it.

Ply is also relevant, https://github.com/crowded/ply/, and we need to do state machine generation as well

The main Readme file says "Tail calls are not optimized".

However it is written more in a defensive manner of restricting responsibility or covering one's back for the situation that could not be solved. It is indeed a good practice to warn that something is not expected to work.

It's not clear however what does work or how to make a workaround for stuff that doesn't.

I would propose to include an example of how a recursive or loop function would have to be written in order not to cause stack or heap overflow in case where there is some main endless async loop function (e.g. something akin to code below):

let asyncApp state = task {
    let! event = waitForNextEventAsync()
    let! newState = updateStateAsync state event
    return asyncApp newState
}

asyncApp initialState

From what's written in Readme, I assume that the above code would leak, but don't really know how to fix it, e.g. would mutable state and a while do loop solve it? What about return, is it necessary at all..

Could you please provide a similar example of code that works and of code that doesn't work with a short explanation of what's actually going on.

Is there an equivalent of Async.RunSynchronously aside from the .Result? I'm thinking as a function that the task can be piped to.

Was porting an old project (netcoreapp2.2) to 3.1 today, noticed something that did compile back then that doesn't anymore.

This code is not sufficiently generic. The type variable ^TEntity when ^TEntity : not struct could not be generalized because it would escape its scope

Note the compiler inferred an SRTP type var ^TEntity here even though no SRTP constraints are used.

open FSharp.Control.Tasks.V2.ContextInsensitive
open Microsoft.EntityFrameworkCore

type DbSet<'TEntity when 'TEntity: not struct> with
    member this.TryFindAsync(keyValues) = task {
        let! r = this.FindAsync(keyValues)
        if obj.ReferenceEquals(r, null) then return ValueNone
        else return ValueSome r
    }

What seems to have happened here is EF moving from returning Task<'T> in 2.x to ValueTask<'T> in 3.x that caused this error to surface.

I've produced a minimal repro

open FSharp.Control.Tasks.V2.ContextInsensitive

type Foo<'T> =
    member this.FindAsync() = ValueTask<_>(Unchecked.defaultof<'T>)
    member this.TryFindAsync() = task {
        let! r = this.FindAsync()
        if obj.ReferenceEquals(r, null) then return ValueNone
        else return ValueSome r
    }

My hunch is something is iffy around the SRTP based 'tasklike' Bind as ValueTask isn't directly supported.

Also, adding inline to the new member TryFindAsync to potentially flow the SRTP var doesn't work either and returns a different error:

The signature and implementation are not compatible because the type parameter in the class/signature has a different compile-time requirement to the one in the member/implementation

Finally to confirm my hunch I tried this code in Ply under netstandard2.0, where Ply has no explicit ValueTask overloads, only similar tasklike support, and under netcoreapp2.2 (which does have the overloads) both compile without errors. Something is going wrong with the SRTP constrained overloads in Taskbuilder.

/cc: @gusty

Alright locally moved the benchmark suite to BDN and these are the results

TaskBuilder is looking good, expected to have a bit more allocs but overall the mean times are very close to C#.

Would you be willing to pull the changes of the setup for BDN and new proj system? Then I'll open a PR :)

Hi,

thank you for this great builder, I've been using it in a few projects.

Do you remember which code constructs caused the type inference issues and made you add unitTask?

The reason I ask is that there was some work recently to improve the type inference in the compiler, so maybe this is already solved. If not, I would like to extract a repro and add an issue at https://github.com/Microsoft/visualfsharp.

In a simple test, everything seemed to work, but I do know that the solver sometimes breaks down when code gets more complex:

module Program

open System.Threading.Tasks
open FSharp.Control.Tasks.ContextSensitive

type X = { A : int ; B : int }

[<EntryPoint>]
let main argv =
    let tX = Task.FromResult({A=1;B=2})
    let t = task {
        do! Task.Delay(100)
        let! x = tX
        return x.B;
    }

    printfn "%i" t.Result

    0

^^^ this works as expected for me, using TaskBuilder.fs from 6aa7ef6

Hi,

Using the latest version of Giraffe, which uses the latest version of TaskBuilder.fs (the NuGet package) I run into an issue where the Dispose() method is not being invoked from inside the task CE.

I have created the following project to reproduce the issue:

DisposableTest.fsproj:

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>
    <DebugType>portable</DebugType>
    <AssemblyName>DisposableTest</AssemblyName>
    <OutputType>Exe</OutputType>
    <RuntimeFrameworkVersion>2.0.0</RuntimeFrameworkVersion>
    <EnableDefaultContentItems>false</EnableDefaultContentItems>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.Hosting" Version="2.0.*" />
    <PackageReference Include="Microsoft.AspNetCore.Diagnostics" Version="2.0.*" />
    <PackageReference Include="Microsoft.AspNetCore.Server.Kestrel" Version="2.0.*" />
    <PackageReference Include="Microsoft.AspNetCore.Server.IISIntegration" Version="2.0.*"/>
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="2.0.*" />
    <PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="2.0.*" />
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="2.0.*" />
    <PackageReference Include="Giraffe" Version="1.0.*" />
  </ItemGroup>

  <ItemGroup>
    <Compile Include="Program.fs" />
  </ItemGroup>

</Project>

Program.fs:

module DisposableTest.App

open System
open Microsoft.AspNetCore.Builder
open Microsoft.AspNetCore.Hosting
open Microsoft.Extensions.Logging
open Microsoft.Extensions.DependencyInjection
open Giraffe

// ---------------------------------
// Types
// ---------------------------------

type DisposableObject (nr : int) =
    let mutable disposed = false
    let output msg = printfn "%s %i" msg nr

    do output "CREATED"

    let cleanup (disposing : bool) =
        if not disposed then
            if disposing then output "DISPOSING"
            disposed <- true
            output "DISPOSED"
        else output "ALREADY DISPOSED"

    interface IDisposable with
        member this.Dispose() =
            cleanup true
            GC.SuppressFinalize this

    override __.Finalize() =
        cleanup(false)

// ---------------------------------
// Web app
// ---------------------------------

let testHandler : HttpHandler =
    fun next ctx ->
        task {
            use x = new DisposableObject 1
            return! text "Hi 1" next ctx
        }

let testHandler2 : HttpHandler =
    fun next ctx ->
        use x = new DisposableObject 2
        text "Hi 2" next ctx

let testHandler3 : HttpHandler =
    fun next ctx ->
        task {
            let x = (new DisposableObject 3) :> IDisposable
            try
                return! text "Hi 3" next ctx
            finally
                x.Dispose()
        }

let webApp =
    choose [
        route "/api" >=> testHandler
        route "/api2" >=> testHandler2
        route "/api3" >=> testHandler3
        setStatusCode 404 >=> text "Not Found" ]

// ---------------------------------
// Error handler
// ---------------------------------

let errorHandler (ex : Exception) (logger : ILogger) =
    logger.LogError(EventId(), ex, "An unhandled exception has occurred while executing the request.")
    clearResponse >=> setStatusCode 500 >=> text ex.Message

// ---------------------------------
// Config and Main
// ---------------------------------

let configureApp (app : IApplicationBuilder) =
    let env = app.ApplicationServices.GetService<IHostingEnvironment>()
    (match env.IsDevelopment() with
    | true  -> app.UseDeveloperExceptionPage()
    | false -> app.UseGiraffeErrorHandler errorHandler)
        .UseGiraffe(webApp)

let configureServices (services : IServiceCollection) =
    services.AddGiraffe() |> ignore

let configureLogging (builder : ILoggingBuilder) =
    let filter (l : LogLevel) = l.Equals LogLevel.Error
    builder.AddFilter(filter).AddConsole().AddDebug() |> ignore

[<EntryPoint>]
let main _ =
    WebHostBuilder()
        .UseKestrel()
        .UseIISIntegration()
        .Configure(Action<IApplicationBuilder> configureApp)
        .ConfigureServices(configureServices)
        .ConfigureLogging(configureLogging)
        .Build()
        .Run()
    0

As you can see I have created a new type called DisposableObject which implements IDisposable according to best practice standards.

When I run the application and visit the following URLs

http://localhost:5000/api
http://localhost:5000/api2
http://localhost:5000/api3

... then I get the following output in the console:

Hosting environment: Production
Content root path: /Users/dustinmoris/Temp/DisposableTest/src/DisposableTest/bin/Debug/netcoreapp2.0/
Now listening on: http://localhost:5000
Application started. Press Ctrl+C to shut down.
CREATED 1
CREATED 2
DISPOSING 2
DISPOSED 2
CREATED 3

Only the second handler where there is no task {} involved seems to properly dispose of the object.

Any ideas why? It is possible that Giraffe is doing something wrong, but we use the task CE normally from within ASP.NET Core without anything custom around it as far as I know.

• TaskBuilder.fs (1.0.1)

• TaskBuilder.fs (1.1)

any ideas? It breaks everything for me

Would it be possible to release a new version of this package that targets netstandard2.0 rather than 1.6? This has some benefits, one of which is that with the recent release of FSharp.Core 4.7, much simpler dependency graphs in Paket.

Is there any way currently to use ValueTask instead of Task?

I work a lot with Task<bool>, whose two possible valid results could be cached. I added a simple logic:

match continuation() with
| Return r ->
    if typedefof<'a> = typedefof<bool> then
      let rb : bool = unbox(box(r))
      if rb then
        methodBuilder.SetResult(unbox(box(TaskUtil.TrueTask)))
      else
        methodBuilder.SetResult(unbox(box(TaskUtil.FalseTask)))
    else
      methodBuilder.SetResult(Task.FromResult(r))
    null

If that was written in C#, then JIT would treat typedefof<'a> = typedefof<bool> as JIT-time constant and completely eliminate branch. Should we use typedefof or typeof for this? Also C#'s version (T)(object)(value) doesn't cause object allocation for value types since JIT is smart enough to recognize such pattern.

Will those JIT optimizations work for the code above? So that the line let rb : bool = unbox(box(r)) doesn't allocate. Or if it does, how to avoid allocations in F# for such a cast? (I will test later myself, just wanted to discuss/review).

Also (not related, but small for a separate issue) I noticed that on the line let methodBuilder = AsyncTaskMethodBuilder<'a Task>() the type AsyncTaskMethodBuilder is mutable struct but here it is stored in an immutable variable. Is this intentional or the thing works now by chance and doesn't use methods that mutate the struct? There are comments in the source about mutability.

https://github.com/fsprojects/FSharpx.Extras/

This library looks useful, but it seems like it doesn't properly handle Tasks that execute synchronously in the thread of the caller, which leads to stack overflows.

The easiest way to show you what I mean is to make a small change to one of your test cases (readFile function, starting on line 22 of Program.fs in BenchmarkFS):

let readFile path =
      task {
            let buffer = Array.zeroCreate bufferSize
            use file = File.OpenRead(path)
            let mutable reading = true
            let mutable n = 0
            while reading && n < 10000 do
                let! countRead = Task.FromResult(1) // file.ReadAsync(buffer, 0, buffer.Length)
                reading <- countRead > 0
                n <- n + 1
        }

All I did was comment out your truly asynchronous task (file.ReadAsync(...)) and replace it with a dummy task (Task.FromResult(1)) that will execute synchronously. Then I added the n counter to ensure that the while loop will iterate enough times to generate the stack overflow (10,000 is enough on my machine).

Unfortunately I don't know enough about CEs or the TPL to suggest whether this is a small bug or a potential flaw in your whole approach.