Should we ever use std::function instead of WTF::Function?

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Should we ever use std::function instead of WTF::Function?

Darin Adler
I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.

Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Chris Dumez
The only advantage of std::function that I know of is that it is copyable. Unless you really need to copy your lambda for a reason or another, I don’t see a good reason to ever use std::function instead of WTF::Function.

I have been slowly replacing std::function by WTF::Function in the code base but there are still a lot of uses of std::function.

--
 Chris Dumez




On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:

I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.

Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo
In reply to this post by Darin Adler
We should have a better story here.  Right now the story is too complicated.  We have:

- ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
- SharedTask if you have a heap-allocated function that you want to share and ref-count
- WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
- std::function if you have a heap-alloated function that you want to pass by copy

Only std::function and WTF::Function do the magic that lets you say:

std::function f = <lambda>

Also, std::function has the benefit that it does copying.  None of the others do that.

ScopedLambda serves a specific purpose: it avoids allocation.  Probably we want to keep that one even if we merge the others.

IMO SharedTask has the nicest semantics.  I don’t ever want the activation of the function to be copied.  In my experience I always want sharing if more than one reference to the function exists.  I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask.  That would let us get rid of std::function and SharedTask.

In the current status quo, it’s not always correct to convert std::function to the others because:

- Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
- Unlike ScopedLambda, std::function is safe if the use is not scoped.
- Unlike WTF::Function, std::function can be copied.

-Filip


> On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:
>
> I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.
>
> Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?
>
> — Darin
> _______________________________________________
> webkit-dev mailing list
> [hidden email]
> https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Chris Dumez
In most cases in WebCore at least, we don’t actually want to share ownership of the lambda so we don’t need RefCounting / SharedTask. Because of this, I don’t think we should merge SharedTask into Function. I think that as it stands, WTF::Function is a suitable replacement for most uses in WebCore since we actually very rarely need copying (either it just builds or the code can be refactored very slightly to avoid the copying).

--
 Chris Dumez




On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:

We should have a better story here.  Right now the story is too complicated.  We have:

- ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
- SharedTask if you have a heap-allocated function that you want to share and ref-count
- WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
- std::function if you have a heap-alloated function that you want to pass by copy

Only std::function and WTF::Function do the magic that lets you say:

std::function f = <lambda>

Also, std::function has the benefit that it does copying.  None of the others do that.

ScopedLambda serves a specific purpose: it avoids allocation.  Probably we want to keep that one even if we merge the others.

IMO SharedTask has the nicest semantics.  I don’t ever want the activation of the function to be copied.  In my experience I always want sharing if more than one reference to the function exists.  I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask.  That would let us get rid of std::function and SharedTask.

In the current status quo, it’s not always correct to convert std::function to the others because:

- Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
- Unlike ScopedLambda, std::function is safe if the use is not scoped.
- Unlike WTF::Function, std::function can be copied.

-Filip


On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:

I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.

Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo
Would SharedTask’s sharing be semantically incorrect for users of WTF::Function?  In other words, do you rely on the compiler error that says that there is no copy constructor?

I’m imagining that if WTF::Function was backed by SharedTask that it would not result in any behavior change for existing WTF::Function users.  At worst, it would mean that WTF::Function’s backing store has an extra word for the ref count - but if you only move and never copy then this word starts out at 1 and stays there until death, so it’s very cheap.

-Filip


On Jun 13, 2017, at 9:43 AM, Chris Dumez <[hidden email]> wrote:

In most cases in WebCore at least, we don’t actually want to share ownership of the lambda so we don’t need RefCounting / SharedTask. Because of this, I don’t think we should merge SharedTask into Function. I think that as it stands, WTF::Function is a suitable replacement for most uses in WebCore since we actually very rarely need copying (either it just builds or the code can be refactored very slightly to avoid the copying).

--
 Chris Dumez




On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:

We should have a better story here.  Right now the story is too complicated.  We have:

- ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
- SharedTask if you have a heap-allocated function that you want to share and ref-count
- WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
- std::function if you have a heap-alloated function that you want to pass by copy

Only std::function and WTF::Function do the magic that lets you say:

std::function f = <lambda>

Also, std::function has the benefit that it does copying.  None of the others do that.

ScopedLambda serves a specific purpose: it avoids allocation.  Probably we want to keep that one even if we merge the others.

IMO SharedTask has the nicest semantics.  I don’t ever want the activation of the function to be copied.  In my experience I always want sharing if more than one reference to the function exists.  I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask.  That would let us get rid of std::function and SharedTask.

In the current status quo, it’s not always correct to convert std::function to the others because:

- Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
- Unlike ScopedLambda, std::function is safe if the use is not scoped.
- Unlike WTF::Function, std::function can be copied.

-Filip


On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:

I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.

Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev



_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Maciej Stachowiak
In reply to this post by Filip Pizlo

In case it turns out not to be possible to reduce the number of concepts (besides eliminating std::function), maybe it would help to change the names and behaviors of these classes to match better. Function, SharedFunction and ScopedFunction would have a much more obvious relationship to each other than Function, SharedTask and ScopedLambda.

(I'm not sure if the direct assignment from a lambda is an incidental difference or one that's required by the different ownership semantics.)

 - Maciej

> On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:
>
> We should have a better story here.  Right now the story is too complicated.  We have:
>
> - ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
> - SharedTask if you have a heap-allocated function that you want to share and ref-count
> - WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
> - std::function if you have a heap-alloated function that you want to pass by copy
>
> Only std::function and WTF::Function do the magic that lets you say:
>
> std::function f = <lambda>
>
> Also, std::function has the benefit that it does copying.  None of the others do that.
>
> ScopedLambda serves a specific purpose: it avoids allocation.  Probably we want to keep that one even if we merge the others.
>
> IMO SharedTask has the nicest semantics.  I don’t ever want the activation of the function to be copied.  In my experience I always want sharing if more than one reference to the function exists.  I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask.  That would let us get rid of std::function and SharedTask.
>
> In the current status quo, it’s not always correct to convert std::function to the others because:
>
> - Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
> - Unlike ScopedLambda, std::function is safe if the use is not scoped.
> - Unlike WTF::Function, std::function can be copied.
>
> -Filip
>
>
>> On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:
>>
>> I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.
>>
>> Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?
>>
>> — Darin
>> _______________________________________________
>> webkit-dev mailing list
>> [hidden email]
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
> _______________________________________________
> webkit-dev mailing list
> [hidden email]
> https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Konstantin Tokarev


13.06.2017, 20:08, "Maciej Stachowiak" <[hidden email]>:
> In case it turns out not to be possible to reduce the number of concepts (besides eliminating std::function), maybe it would help to change the names and behaviors of these classes to match better. Function, SharedFunction and ScopedFunction would have a much more obvious relationship to each other than Function, SharedTask and ScopedLambda.

Maybe rename Function to UniqueFunction?

>
> (I'm not sure if the direct assignment from a lambda is an incidental difference or one that's required by the different ownership semantics.)
>
>  - Maciej
>
>>  On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:
>>
>>  We should have a better story here. Right now the story is too complicated. We have:
>>
>>  - ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
>>  - SharedTask if you have a heap-allocated function that you want to share and ref-count
>>  - WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
>>  - std::function if you have a heap-alloated function that you want to pass by copy
>>
>>  Only std::function and WTF::Function do the magic that lets you say:
>>
>>  std::function f = <lambda>
>>
>>  Also, std::function has the benefit that it does copying. None of the others do that.
>>
>>  ScopedLambda serves a specific purpose: it avoids allocation. Probably we want to keep that one even if we merge the others.
>>
>>  IMO SharedTask has the nicest semantics. I don’t ever want the activation of the function to be copied. In my experience I always want sharing if more than one reference to the function exists. I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask. That would let us get rid of std::function and SharedTask.
>>
>>  In the current status quo, it’s not always correct to convert std::function to the others because:
>>
>>  - Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
>>  - Unlike ScopedLambda, std::function is safe if the use is not scoped.
>>  - Unlike WTF::Function, std::function can be copied.
>>
>>  -Filip
>>
>>>  On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:
>>>
>>>  I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.
>>>
>>>  Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?
>>>
>>>  — Darin
>>>  _______________________________________________
>>>  webkit-dev mailing list
>>>  [hidden email]
>>>  https://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>>  _______________________________________________
>>  webkit-dev mailing list
>>  [hidden email]
>>  https://lists.webkit.org/mailman/listinfo/webkit-dev
>
> _______________________________________________
> webkit-dev mailing list
> [hidden email]
> https://lists.webkit.org/mailman/listinfo/webkit-dev

--
Regards,
Konstantin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo
I like these renamings. I’m also ok with renaming Function to UniqueFunction but I’m also ok with keeping the current name.

-Filip

> On Jun 13, 2017, at 10:12 AM, Konstantin Tokarev <[hidden email]> wrote:
>
>
>
> 13.06.2017, 20:08, "Maciej Stachowiak" <[hidden email]>:
>> In case it turns out not to be possible to reduce the number of concepts (besides eliminating std::function), maybe it would help to change the names and behaviors of these classes to match better. Function, SharedFunction and ScopedFunction would have a much more obvious relationship to each other than Function, SharedTask and ScopedLambda.
>
> Maybe rename Function to UniqueFunction?
>
>>
>> (I'm not sure if the direct assignment from a lambda is an incidental difference or one that's required by the different ownership semantics.)
>>
>>  - Maciej
>>
>>>  On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:
>>>
>>>  We should have a better story here. Right now the story is too complicated. We have:
>>>
>>>  - ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
>>>  - SharedTask if you have a heap-allocated function that you want to share and ref-count
>>>  - WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
>>>  - std::function if you have a heap-alloated function that you want to pass by copy
>>>
>>>  Only std::function and WTF::Function do the magic that lets you say:
>>>
>>>  std::function f = <lambda>
>>>
>>>  Also, std::function has the benefit that it does copying. None of the others do that.
>>>
>>>  ScopedLambda serves a specific purpose: it avoids allocation. Probably we want to keep that one even if we merge the others.
>>>
>>>  IMO SharedTask has the nicest semantics. I don’t ever want the activation of the function to be copied. In my experience I always want sharing if more than one reference to the function exists. I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask. That would let us get rid of std::function and SharedTask.
>>>
>>>  In the current status quo, it’s not always correct to convert std::function to the others because:
>>>
>>>  - Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
>>>  - Unlike ScopedLambda, std::function is safe if the use is not scoped.
>>>  - Unlike WTF::Function, std::function can be copied.
>>>
>>>  -Filip
>>>
>>>>  On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:
>>>>
>>>>  I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.
>>>>
>>>>  Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?
>>>>
>>>>  — Darin
>>>>  _______________________________________________
>>>>  webkit-dev mailing list
>>>>  [hidden email]
>>>>  https://lists.webkit.org/mailman/listinfo/webkit-dev
>>>
>>>  _______________________________________________
>>>  webkit-dev mailing list
>>>  [hidden email]
>>>  https://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>> _______________________________________________
>> webkit-dev mailing list
>> [hidden email]
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
> --
> Regards,
> Konstantin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Brady Eidson
In reply to this post by Filip Pizlo

On Jun 13, 2017, at 9:55 AM, Filip Pizlo <[hidden email]> wrote:

Would SharedTask’s sharing be semantically incorrect for users of WTF::Function?  In other words, do you rely on the compiler error that says that there is no copy constructor?

WTF::Function is used in cross-thread dispatching, where arguments captured by the lambda are “thread safe copied”, “isolated copied”, etc.

Part of the safety is the lack of a copy constructor, as accidentally making a copy on the originating thread can invalidate the setup for thread safety.

So, yes, they must maintain move-only semantics.

I’m imagining that if WTF::Function was backed by SharedTask that it would not result in any behavior change for existing WTF::Function users.

I see the reverse. Is there any reason SharedTask can’t be backed by a WTF::Function?

There’s no harm in adding Ref counting semantics on top the move-only WTF::Function if SharedTask is your goal.

Thanks,
 Brady

 At worst, it would mean that WTF::Function’s backing store has an extra word for the ref count - but if you only move and never copy then this word starts out at 1 and stays there until death, so it’s very cheap.

-Filip


On Jun 13, 2017, at 9:43 AM, Chris Dumez <[hidden email]> wrote:

In most cases in WebCore at least, we don’t actually want to share ownership of the lambda so we don’t need RefCounting / SharedTask. Because of this, I don’t think we should merge SharedTask into Function. I think that as it stands, WTF::Function is a suitable replacement for most uses in WebCore since we actually very rarely need copying (either it just builds or the code can be refactored very slightly to avoid the copying).

--
 Chris Dumez




On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:

We should have a better story here.  Right now the story is too complicated.  We have:

- ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
- SharedTask if you have a heap-allocated function that you want to share and ref-count
- WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
- std::function if you have a heap-alloated function that you want to pass by copy

Only std::function and WTF::Function do the magic that lets you say:

std::function f = <lambda>

Also, std::function has the benefit that it does copying.  None of the others do that.

ScopedLambda serves a specific purpose: it avoids allocation.  Probably we want to keep that one even if we merge the others.

IMO SharedTask has the nicest semantics.  I don’t ever want the activation of the function to be copied.  In my experience I always want sharing if more than one reference to the function exists.  I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask.  That would let us get rid of std::function and SharedTask.

In the current status quo, it’s not always correct to convert std::function to the others because:

- Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
- Unlike ScopedLambda, std::function is safe if the use is not scoped.
- Unlike WTF::Function, std::function can be copied.

-Filip


On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:

I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.

Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo

On Jun 13, 2017, at 11:56 AM, Brady Eidson <[hidden email]> wrote:


On Jun 13, 2017, at 9:55 AM, Filip Pizlo <[hidden email]> wrote:

Would SharedTask’s sharing be semantically incorrect for users of WTF::Function?  In other words, do you rely on the compiler error that says that there is no copy constructor?

WTF::Function is used in cross-thread dispatching, where arguments captured by the lambda are “thread safe copied”, “isolated copied”, etc.

Part of the safety is the lack of a copy constructor, as accidentally making a copy on the originating thread can invalidate the setup for thread safety.

So, yes, they must maintain move-only semantics.

It seems that the benefit of keeping existing Function semantics is that it will sometimes prevent you from some races (but not all since as you say, you can still put the Function in a shared object).

The benefit of merging Function and SharedTask is twofold:

1) JSC uses SharedTask in a lot of places not because we want to actually share it with many threads, but because RefPtr<> can be stored in a lot more kinds of data structures.  I think there are places where we do some transformation on a set of tasks, and implementing it all using move semantics would be cumbersome.  For example, most textbook sorting algorithms are most easily implemented if you can do “=“ and temporarily create aliases (or copies).  This is the main reason why I find myself switching Function code to use SharedTask.

2) You won’t need a different type when you do want sharing between threads.  I wrote SharedTask initially for this purpose, but this is usually not the reason why I use it.

I get that being able to guarantee that (2) won’t happen is attractive, but I’m worried that using no-copy to ensure this makes other things harder.


I’m imagining that if WTF::Function was backed by SharedTask that it would not result in any behavior change for existing WTF::Function users. 

I see the reverse. Is there any reason SharedTask can’t be backed by a WTF::Function?

There’s no harm in adding Ref counting semantics on top the move-only WTF::Function if SharedTask is your goal.

My goal is to see if we can merge SharedTask and Function.  I don’t have opinions on how the two should be implemented.

-Filip



Thanks,
 Brady

 At worst, it would mean that WTF::Function’s backing store has an extra word for the ref count - but if you only move and never copy then this word starts out at 1 and stays there until death, so it’s very cheap.

-Filip


On Jun 13, 2017, at 9:43 AM, Chris Dumez <[hidden email]> wrote:

In most cases in WebCore at least, we don’t actually want to share ownership of the lambda so we don’t need RefCounting / SharedTask. Because of this, I don’t think we should merge SharedTask into Function. I think that as it stands, WTF::Function is a suitable replacement for most uses in WebCore since we actually very rarely need copying (either it just builds or the code can be refactored very slightly to avoid the copying).

--
 Chris Dumez




On Jun 13, 2017, at 9:34 AM, Filip Pizlo <[hidden email]> wrote:

We should have a better story here.  Right now the story is too complicated.  We have:

- ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that outlives its user
- SharedTask if you have a heap-allocated function that you want to share and ref-count
- WTF::Function if you have a heap-allocated function that you want to transfer ownership (move yes, copy no)
- std::function if you have a heap-alloated function that you want to pass by copy

Only std::function and WTF::Function do the magic that lets you say:

std::function f = <lambda>

Also, std::function has the benefit that it does copying.  None of the others do that.

ScopedLambda serves a specific purpose: it avoids allocation.  Probably we want to keep that one even if we merge the others.

IMO SharedTask has the nicest semantics.  I don’t ever want the activation of the function to be copied.  In my experience I always want sharing if more than one reference to the function exists.  I think that what we really want in most places is a WTF::Function that has sharing semantics like SharedTask.  That would let us get rid of std::function and SharedTask.

In the current status quo, it’s not always correct to convert std::function to the others because:

- Unlike ScopedLambda and SharedTask, std::function has the magic constructor that allows you to just assign a lambda to it.
- Unlike ScopedLambda, std::function is safe if the use is not scoped.
- Unlike WTF::Function, std::function can be copied.

-Filip


On Jun 13, 2017, at 9:24 AM, Darin Adler <[hidden email]> wrote:

I’ve noticed many patches switching us from std::function to WTF::Function recently, to fix problems with copying and thread safety.

Does std::function have any advantages over WTF::Function? Should we ever prefer std::function, or should we use WTF::Function everywhere in WebKit where we would otherwise use std::function?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Alex Christensen-2
In reply to this post by Brady Eidson
std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo
And ObjC blocks share instead of copying, which is more like SharedTask.  I think that’s another reason why adding copy constructors to WTF::Function that do block-like sharing under the hood is rather attractive.

-Filip


> On Jun 13, 2017, at 12:29 PM, Alex Christensen <[hidden email]> wrote:
>
> std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Chris Dumez
In reply to this post by Alex Christensen-2
We already have BlockPtr for passing a Function as a lambda block.

Chris Dumez

On Jun 13, 2017, at 12:29 PM, Alex Christensen <[hidden email]> wrote:

std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Alex Christensen-2
Ok, maybe we can get rid of std::function, then!  I hadn’t used BlockPtr as much as Chris.  I’d be opposed to adding a copy constructor to WTF::Function because the non-copyability of WTF::Function is why we made it, and it has prevented many bugs.

I’ve also seen many cases where I have a WTF::Function that I want to make sure is called once and only once before destruction.  I wouldn’t mind adding a WTF::Callback subclass that just asserts that it has been called once.  That would’ve prevented some bugs, too, but not every use of WTF::Function has such a requirement.

On Jun 13, 2017, at 12:31 PM, Chris Dumez <[hidden email]> wrote:

We already have BlockPtr for passing a Function as a lambda block.

Chris Dumez

On Jun 13, 2017, at 12:29 PM, Alex Christensen <[hidden email]> wrote:

std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo

On Jun 13, 2017, at 12:37 PM, Alex Christensen <[hidden email]> wrote:

Ok, maybe we can get rid of std::function, then!  I hadn’t used BlockPtr as much as Chris.  I’d be opposed to adding a copy constructor to WTF::Function because the non-copyability of WTF::Function is why we made it, and it has prevented many bugs.

I agree that the copy semantics of std::function are strange - each copy gets its own view of the state of the closure.  This gets super weird when you implement an algorithm that accidentally copies the function - the act of copying invokes copy constructors on all of the lambda-lifted state, which is probably not what you wanted.  So I’m not surprised that moving to WTF::Function avoided bugs.  What I’m proposing also prevents many of the same bugs because the lambda-lifted state never gets copied in my world.

Do you think that code that uses ObjC blocks encounters the kind of bugs that you saw WTF::Function preventing?  Or are the bugs that Function prevents more specific to std::function?  I guess I’d never heard of a need to change block semantics to avoid bugs, so I have a hunch that the bugs you guys prevented were specific to the fact that std::function copies instead of sharing.


I’ve also seen many cases where I have a WTF::Function that I want to make sure is called once and only once before destruction.  I wouldn’t mind adding a WTF::Callback subclass that just asserts that it has been called once.  That would’ve prevented some bugs, too, but not every use of WTF::Function has such a requirement.

On Jun 13, 2017, at 12:31 PM, Chris Dumez <[hidden email]> wrote:

We already have BlockPtr for passing a Function as a lambda block.

Chris Dumez

On Jun 13, 2017, at 12:29 PM, Alex Christensen <[hidden email]> wrote:

std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Chris Dumez

On Jun 13, 2017, at 12:51 PM, Filip Pizlo <[hidden email]> wrote:


On Jun 13, 2017, at 12:37 PM, Alex Christensen <[hidden email]> wrote:

Ok, maybe we can get rid of std::function, then!  I hadn’t used BlockPtr as much as Chris.  I’d be opposed to adding a copy constructor to WTF::Function because the non-copyability of WTF::Function is why we made it, and it has prevented many bugs.

I agree that the copy semantics of std::function are strange - each copy gets its own view of the state of the closure.  This gets super weird when you implement an algorithm that accidentally copies the function - the act of copying invokes copy constructors on all of the lambda-lifted state, which is probably not what you wanted.  So I’m not surprised that moving to WTF::Function avoided bugs.  What I’m proposing also prevents many of the same bugs because the lambda-lifted state never gets copied in my world.

I understand SharedTask avoids many of the same issues. My issue is that it adds an extra data member for refcounting that is very rarely needed in practice. Also, because this type is copyable, people can create refcounting churn by inadvertently copying.
Because most of the time, we do not want to share and WTF::Function is sufficient as it stands, I do not think it is worth making WTF::Function refcounted.


Do you think that code that uses ObjC blocks encounters the kind of bugs that you saw WTF::Function preventing?  Or are the bugs that Function prevents more specific to std::function?  I guess I’d never heard of a need to change block semantics to avoid bugs, so I have a hunch that the bugs you guys prevented were specific to the fact that std::function copies instead of sharing.

To be cleared, we viewed this as “copies instead of truly moving”, not sharing. We never really needed sharing when WTF::Function was initially called WTF::NonCopyableFunction.
Yes, the bugs we were trying to avoid were related to using std::function and copying things implicitly, even if you WTFMove() it around. Because we started using WTF::Function instead of std::function in more places though, having BlockPtr::fromCallable() to be able to pass a WTF::Function to an ObjC function expecting a block became handy though. 



I’ve also seen many cases where I have a WTF::Function that I want to make sure is called once and only once before destruction.  I wouldn’t mind adding a WTF::Callback subclass that just asserts that it has been called once.  That would’ve prevented some bugs, too, but not every use of WTF::Function has such a requirement.

On Jun 13, 2017, at 12:31 PM, Chris Dumez <[hidden email]> wrote:

We already have BlockPtr for passing a Function as a lambda block.

Chris Dumez

On Jun 13, 2017, at 12:29 PM, Alex Christensen <[hidden email]> wrote:

std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Filip Pizlo

On Jun 13, 2017, at 1:00 PM, Chris Dumez <[hidden email]> wrote:


On Jun 13, 2017, at 12:51 PM, Filip Pizlo <[hidden email]> wrote:


On Jun 13, 2017, at 12:37 PM, Alex Christensen <[hidden email]> wrote:

Ok, maybe we can get rid of std::function, then!  I hadn’t used BlockPtr as much as Chris.  I’d be opposed to adding a copy constructor to WTF::Function because the non-copyability of WTF::Function is why we made it, and it has prevented many bugs.

I agree that the copy semantics of std::function are strange - each copy gets its own view of the state of the closure.  This gets super weird when you implement an algorithm that accidentally copies the function - the act of copying invokes copy constructors on all of the lambda-lifted state, which is probably not what you wanted.  So I’m not surprised that moving to WTF::Function avoided bugs.  What I’m proposing also prevents many of the same bugs because the lambda-lifted state never gets copied in my world.

I understand SharedTask avoids many of the same issues. My issue is that it adds an extra data member for refcounting that is very rarely needed in practice. Also, because this type is copyable, people can create refcounting churn by inadvertently copying.
Because most of the time, we do not want to share and WTF::Function is sufficient as it stands, I do not think it is worth making WTF::Function refcounted.

Perf is a good argument against this, but then it seems like WTF::Function should by default allow block-style sharing semantics, and then there would be a WTF::UniqueFunction that is more optimal.

Also, who is “we” here?  If “we” includes JSC, then “we” don’t find WTF::Function sufficient.  I try to use WTF::Function whenever I can because I like the name and I like the API, but often I have to switch to SharedTask in order to make the code work.  B3’s use of SharedTask for its StackmapGenerator is a great example.  We can’t use WTF::Function in that code because the data structures that reference the generator get copied and that’s OK when we use SharedTask.



Do you think that code that uses ObjC blocks encounters the kind of bugs that you saw WTF::Function preventing?  Or are the bugs that Function prevents more specific to std::function?  I guess I’d never heard of a need to change block semantics to avoid bugs, so I have a hunch that the bugs you guys prevented were specific to the fact that std::function copies instead of sharing.

To be cleared, we viewed this as “copies instead of truly moving”, not sharing. We never really needed sharing when WTF::Function was initially called WTF::NonCopyableFunction.
Yes, the bugs we were trying to avoid were related to using std::function and copying things implicitly, even if you WTFMove() it around. Because we started using WTF::Function instead of std::function in more places though, having BlockPtr::fromCallable() to be able to pass a WTF::Function to an ObjC function expecting a block became handy though. 



I’ve also seen many cases where I have a WTF::Function that I want to make sure is called once and only once before destruction.  I wouldn’t mind adding a WTF::Callback subclass that just asserts that it has been called once.  That would’ve prevented some bugs, too, but not every use of WTF::Function has such a requirement.

On Jun 13, 2017, at 12:31 PM, Chris Dumez <[hidden email]> wrote:

We already have BlockPtr for passing a Function as a lambda block.

Chris Dumez

On Jun 13, 2017, at 12:29 PM, Alex Christensen <[hidden email]> wrote:

std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev



_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Should we ever use std::function instead of WTF::Function?

Brady Eidson
In reply to this post by Alex Christensen-2

> On Jun 13, 2017, at 12:37 PM, Alex Christensen <[hidden email]> wrote:
>
> I’d be opposed to adding a copy constructor to WTF::Function because the non-copyability of WTF::Function is why we made it, and it has prevented many bugs.

Ditto.

The lack of a copy c’tor in WTF::Function turns the compiler into a tool that prevents bugs that engineers would’ve otherwise introduced.

Whatever the goal of the refactoring is here, for me it’s an absolute deal breaker for us to end up without a <function type> that is move-only.

Thanks,
 Brady

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Loading...