I guess that means a no then. CL closed. On Wednesday, May 17, 2017 at 3:23:50 PM UTC+8, Jochen Eisinger wrote: > > 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] <javascript:>> 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] <javascript:> >> 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] <javascript:>. >> 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.
