rspeele / TaskBuilder.fs

F# computation expression builder for System.Threading.Tasks
Creative Commons Zero v1.0 Universal
235 stars 27 forks source link

Caching bool tasks #21

Open buybackoff opened 6 years ago

buybackoff commented 6 years ago

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.

rspeele commented 6 years ago

I have to agree with your commentary on the fslang-suggestions topic that Task-building is best implemented in the compiler (i.e. not by me), just like in C#, so optimizations like this can be done without runtime penalty.

If you check the IL, F#'s typedefof is pretty inefficient. C#'s typeof(X<>) compiles straight to a ldtoken, while F#'s typedefof<X<_>> is the same as writing typeof<X<obj>>.GetGenericTypeDefinition(). At least, that's how it was last time I checked.

The methodbuilder should probably be declared mutable as you point out, but I don't think it being "immutable" in the compiler's option actually prevents mutation in this case. Where I'd be concerned is the let-bound method that references it getting a copy instead of closing over the field, but I think that ends up as an instance method of the class so it gets to play with the real field.

ForNeVeR commented 6 years ago

If you check the IL, F#'s typedefof is pretty inefficient.

I believe we can fix that!