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

Caching bool tasks #21

Open
buybackoff opened this issue May 22, 2018 · 2 comments
Open

Caching bool tasks #21

buybackoff opened this issue May 22, 2018 · 2 comments

Comments

@buybackoff
Copy link

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

rspeele commented May 27, 2018

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

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

I believe we can fix that!

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

3 participants