Hi Mostyn, this is awesome, I'm very excited about it! Thank you for working on this. A few comments:
I see you've added a few #pragmas around existing "using namespace v8" directives. Those should just be removed entirely, they're a style guide violation anyway. That burden should not be on this CL though, so I'm fine with the pragmas as a temporary workaround. I'm not a fan of the "namespace ANON" solution. I think we should just rename stuff as necessary to avoid name clashes. In that case we could even keep the anonymous namespaces (to make the linker's life easier by hiding those symbols from it), right? Or are they a problem for jumbo builds in general? Instead of the sequence: #undef foo #define foo bar // use foo here I would prefer if we maintained good hygiene and consistently did: #define foo // use foo here #undef foo If it's too difficult to find all places where we forget to "#undef __", let me help you find them! What are the reasons why you have to skip a few files via jumbo_excluded_sources? I.e. what would it take to get them un-blacklisted? And one final nit: Thanks for finding and adding all those missing include guards! V8's header include guards begin with "V8_", e.g. V8_FILENAME_H. This is to prevent name clashes with Chromium ;-) Thanks, Jakob On Thu, Jul 20, 2017 at 3:46 PM, <[email protected]> wrote: > Following on from the thread in blink-dev and chromium-dev[1], I have a > preliminary patch that enabled jumbo v8 builds: > https://chromium-review.googlesource.com/c/579090/ > > In my test setup (an aging pc with 4core/8thread cpu, 32G of ram and an > SSD, not using ccache or icecc), I am able to build v8 72% faster than a > regular non-jumbo build. > > I think most of this CL is uncontroversial (rename some symbols, redefine > commonly used macros and so on), however I would like opinions on the best > way to handle anonymous namespaces, which in jumbo builds are not > file-local. In this initial patch, the resolution I am using is to > redefine an "ANON" macro to a unique name corresponding to the source file, > name the formerly anonymous namespace with that macro, and then add some > using ANON::foo statements outside the namespace. This requires a #pragma > workaround for clang when using -Wheader-hygiene and -Werror. Perhaps it > would be better to just change these into regular named namespaces and > specify the namespace when using the symbols? Or just drop anonymous > namespaces altogether? > > [1] https://groups.google.com/a/chromium.org/d/msg/chromium- > dev/B69B105btQg/JWCB2WOdBwAJ > > -Mostyn. > -- > Mostyn Bramley-Moore > Opera TV > [email protected] > > -- > -- > 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.
