While I can see the benefits of your proposed approach, I'm opposed to taking in a class that isn't either maintained by chromium or in the STL. We don't have the bandwidth to maintain a custom wrapper like function_view ourselves at this point.
On Wed, May 17, 2017 at 6:36 AM <[email protected]> wrote: > I updated some more codes to function_view (pushed to CL so you can see). > > I compare the binaries built from local master branch with current CL > branch. Even though I have not complete the refactoring yet, size of d8.exe > and mksnapshot.exe already reduced by 5KB and 15KB respectively. > > function_view's implementation has some problem with non-static class > methods, so I am going to change my plan a bit. > > My current idea is: > > - If normal function *is passed to* another function as parameter, use > raw function pointer (as done in some V8 codes). > - If lambda function with capture and/or normal function *is passed to* > another function as parameter, use function_view. > - For other case when you need to pass/*store* normal function, lambda > function and non-static class methods, use std::function. > > I might port Chromium's Callback<> to V8 for CodeAssemblerCallback, but > that will be a separated CL because the work is non-trivial for me. Looking > at he implementation of Callback, I think it is almost as complex as (if > not more than) std:function, so it isn't exactly lightweight. > > What do you think? > > On Tuesday, May 16, 2017 at 9:43:02 PM UTC+8, [email protected] wrote: >> >> On Tuesday, May 16, 2017 at 9:13:32 PM UTC+8, Jochen Eisinger wrote: >>> >>> I'd vote for either importing chromium's base version, or sticking with >>> the pure C++ version >>> >> >> I can try to look for more V8 code that can be benefited from Callback<>. >> In my opinion Callable<>'s syntax is quite different from std::function or >> function_view. >> >> std::function was used in the first case only because they need to use >> lambda function. Most V8 codes just use raw function pointer directly (e.g: >> https://cs.chromium.org/chromium/src/v8/src/codegen.h?type=cs&l=97) >> >> Docs: >> https://chromium.googlesource.com/chromium/src/+/master/docs/callback.md >> >> >>> >>> On Tue, May 16, 2017 at 3:12 PM <[email protected]> wrote: >>> >>>> On Tuesday, May 16, 2017 at 9:05:41 PM UTC+8, Camillo Bruni wrote: >>>>> >>>>> FYI, the code in src/builtin/builtin-*-gen.cc is no longer shipped >>>>> and is neither performance critical as it is only used to create the code >>>>> stubs for the snapshot. >>>>> >>>> >>>> I am aware of that. I noted that in one comment of the CL, but forgot >>>> to re-mention it again in this thread. src/runtime/runtime-regexp.cc and >>>> other codes also use a bit of std::function + lambda, so this refactoring >>>> will still benefit snapshot build slightly. >>>> >>>> By the way, Official Node.js binary is built with snapshot or nosnaphot >>>> configuration? >>>> >>>> >>>>> >>>>> On Tue, May 16, 2017 at 2:52 PM <[email protected]> wrote: >>>>> >>>> On Tuesday, May 16, 2017 at 8:34:43 PM UTC+8, Jochen Eisinger wrote: >>>>>>> >>>>>>> Thanks for the summary! >>>>>>> >>>>>>> I'm wondering whether you also plan other refactoring work for V8? >>>>>>> >>>>>> >>>>>> No, I only have plan for this refactoring for now. >>>>>> >>>>>> >>>>>>> >>>>>>> Also, why not just import base::Bind and friends from Chromium? >>>>>>> >>>>>> >>>>>> Chromium's implementation uses many other Chromium-specific features >>>>>> like weak_ptr<>, we might end up introducing more code than V8 actually >>>>>> needs. As far as I can see, CodeAssemblerCallback seems to be the only >>>>>> use >>>>>> case of std::function that might benefit from Chromium's Callable<>. >>>>>> >>>>>> >>>>>>> >>>>>>> On Tue, May 16, 2017 at 2:04 PM <[email protected]> wrote: >>>>>>> >>>>>>>> Hi V8 devs, >>>>>>>> >>>>>>>> I saw a few functions in src/builtin/builtin-*-gen.cc and >>>>>>>> src/code-stub-assembler.h uses quite a few std::function as a >>>>>>>> parameter, >>>>>>>> then pass lambda function to these functions to avoid code duplication >>>>>>>> as >>>>>>>> much as possible. std::function was used because raw function pointer >>>>>>>> can't >>>>>>>> accept lambda function. >>>>>>>> >>>>>>>> However, because std::function is really powerful, it comes with a >>>>>>>> cost in terms of code size and overhead when calling std::function. >>>>>>>> >>>>>>>> I am working on introducing function_view to src/base, a >>>>>>>> lightweight callable reference that can accept normal and lambda >>>>>>>> function. >>>>>>>> >>>>>>>> function_view is adapted from [1] >>>>>>>> <https://vittorioromeo.info/index/blog/passing_functions_to_functions.html>. >>>>>>>> I have changed it so that it can be compiled with C++11, which is the >>>>>>>> minimum compiler support requirement to build Chromium. >>>>>>>> >>>>>>>> Note: >>>>>>>> >>>>>>>> The callable itself must be alive longer or equal to the life time >>>>>>>> of function_view object. Therefore, function_view is not a universal >>>>>>>> replacement for the general-purpose std::function. >>>>>>>> >>>>>>>> If you need to store lambda function as class property, >>>>>>>> std::function is still needed. Use case like [2] >>>>>>>> <https://cs.chromium.org/chromium/src/v8/src/compiler/code-assembler.h?type=cs&l=601> >>>>>>>> cannot be replaced with function_view. >>>>>>>> >>>>>>>> Chromium do have a very powerful library to store callback at >>>>>>>> https://cs.chromium.org/chromium/src/base/bind_internal.h >>>>>>>> (std::function is banned in Chromium). >>>>>>>> >>>>>>>> Plan: >>>>>>>> >>>>>>>> - Add base::function_view implementation to >>>>>>>> src/base/functional.h >>>>>>>> - Convert std::function to base::function_view in >>>>>>>> src/builtin/builtin-*-gen.cc and maybe other parts of src/* if >>>>>>>> appropriate. >>>>>>>> Usage of std::function in test/* will be ignored. >>>>>>>> - Add documentation. >>>>>>>> - Add unittest. >>>>>>>> >>>>>>>> src/compiler/s390/instruction-selector-s390.cc also uses a lot of >>>>>>>> std::function + lambda, but because I can't test on this platform >>>>>>>> myself, I >>>>>>>> don't feel safe to change it. It looks like dry run does not have bot >>>>>>>> for >>>>>>>> s390. >>>>>>>> >>>>>>>> WIP CL: >>>>>>>> >>>>>>>> https://chromium-review.googlesource.com/c/505653/ >>>>>>>> >>>>>>>> [1]: >>>>>>>> https://vittorioromeo.info/index/blog/passing_functions_to_functions.html >>>>>>>> [2]: >>>>>>>> https://cs.chromium.org/chromium/src/v8/src/compiler/code-assembler.h?type=cs&l=601 >>>>>>>> >>>>>>>> Sincerely, >>>>>>>> Rong Jie >>>>>>>> >>>>>>> -- >>>>>> -- >>>>>> v8-dev mailing list >>>>>> >>>>> [email protected] >>>>> >>>>> >>>>>> http://groups.google.com/group/v8-dev >>>>>> --- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "v8-dev" group. >>>>>> >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>>> an email to [email protected]. >>>>> >>>>> >>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>> >>>>> -- >>>>> Camillo Bruni | Software Engineer, V8 | Google Germany GmbH | >>>>> Erika-Mann >>>>> Str. 33, 80636 München >>>>> <https://maps.google.com/?q=Erika-Mann+Str.+33,+80636+M%C3%BCnchen&entry=gmail&source=g> >>>>> >>>>> Registergericht und -nummer: Hamburg, HRB 86891 | Sitz der >>>>> Gesellschaft: Hamburg | Geschäftsführer: Matthew Scott Sucherman, >>>>> Paul Terence Manicle >>>>> >>>> -- >>>> -- >>>> v8-dev mailing list >>>> [email protected] >>>> http://groups.google.com/group/v8-dev >>>> --- >>>> You received this message because you are subscribed to the Google >>>> Groups "v8-dev" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> -- > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
