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.
