> On Jun 13, 2017, at 1:00 PM, Chris Dumez <cdu...@apple.com> wrote: > > >> On Jun 13, 2017, at 12:51 PM, Filip Pizlo <fpi...@apple.com >> <mailto:fpi...@apple.com>> wrote: >> >>> >>> On Jun 13, 2017, at 12:37 PM, Alex Christensen <achristen...@apple.com >>> <mailto:achristen...@apple.com>> 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 <cdu...@apple.com >>>> <mailto:cdu...@apple.com>> 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 <achristen...@apple.com >>>> <mailto:achristen...@apple.com>> 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 >>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> >>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>> >>> _______________________________________________ >>> webkit-dev mailing list >>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> >>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
