[Intent to implement] Stripping whitespace and comments from Chrome's resources

[Anthony Berent]

Contact

abe...@chromium.org

Summary

The Chrome PAK files includes inlined versions of a large numbers of HTML, CSS, and Javascript files. Many of these files contain extensive comments and whitespace, hence increasing the download and on disk size all versions of Chrome for no benefit. This project will implement changes to the build system to strip whitespace and comments from these files.

Tracking bug

https://crbug/619091

Design doc

https://docs.google.com/document/d/1XUte4m_wkPwQ9I-MfVZfBhksoL0E2_7Eb7N-PkXQKvw/edit?usp=sharing

[PhistucK]

What about minification?

☆PhistucK

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Anthony Berent]

I think this is largely covered by the document (see the discussion in the caveats section). Minification of local variables etc. is probably possible, but doesn't seem to make much difference and makes debugging of Javascript more difficult. Global minification would be more difficult since we would need to give the compiler considerable extra information to ensure that it kept the right, externally accessed, functions and variables. It would also make debugging almost impossible without some extra tools.

[PhistucK]

Sorry, I did not have access to Google Docs until now.

Perhaps consult with the Developer Tools team. While they have not minified the code yet, I believe, they are making sure it has all of the needed externs and could probably be minified with minor effort by now.

Since every kilobyte matters and minification (especially a global one) can squeeze a lot here, I think the extra effort is worth it. It will also make it easier to reason about the code.

Regarding debugging, you can generate source maps and place them on a remote server, a bit like (but I guess not exactly the same as) the Developer Tools is currently stored on a remote server for debugging pages viewed using Chrome for Android. With the new experimental variable resolution in the Developer Tools, you can have a pretty solid debugging experience.

A bonus benefit is that any buggy debugging behavior you discover can be fixed in the Developer Tools or V8 and all of the web developers that minify their code and use source map will benefit. :)

☆PhistucK

[Anthony Berent]

On further minification my preference would be to do one step at a time. Nothing in my current work prevents us later changing the compiler options to do full minification, if we can, as you suggest, finish annotating the code and solve the debugging problems.

I do suspect that there is some way to go on annotating the code. From a quick grep currently only 62 files in Chrome contain "@extern" statements, and, having looked so-far only at the browser and component resources. I know of at least 160 files Javascript files that added to resources.pak. Not all of these will need @extern statements, but I would be surprised if it was under half of them that did.

[PhistucK]

Well, it depends on the granularity. Since the scripts mostly interact with chrome.send, not a lot of externs are needed for methods. For types that are returned from the browser, yes, it would be good to have externs.

Starting with the simple optimization would be beneficial. And not only for Android.

Those comments always seemed like a waste to me. I am glad something is done about those.

Good luck, anyway!

☆PhistucK

[Elliott Sprehn]

+kouhei who is working on doing this for the html.css file in blink. Seems you should share code there. :)

[Brett Wilson]

The concern I had in the doc is that I don't like this part of the change:

https://codereview.chromium.org/2094193004/diff/120001/chrome/browser/BUILD.gn

Here, we're duplicating the listing of the .js files from browser_resources.grd and other random grit files like ui/webui/resources/webui_resources.grd in chrome/browser/BUILD.gn. I feel like this is complicated and fragile to maintain, even more so since this list will affect Android only.

I suggested on the doc that we add a "filter" to grit to process and js files through something as they're being added to the compiled resources. With this design it would run on all .js files in a grd when anything in that .grd has changed. Apparently these resources are changed about once a day, and the Closure compiler is so slow this will add 2 minutes to a build where any resource file has changed. Clearly this is Not Good.

But even a 2 minute regression in full build times I think is pretty bad (which from my reading is what we'll get with the current proposal). Full builds for developer and bots aren't that uncommon.

What about doing the filter idea but only doing this whole process for official builds only? This sort of maps how the optimization levels work anyway, and means that even release builds that developers might use are easy to debug. The official builders often build all anyway (I think only perf testers might build official incrementally) so the build-time impact should not be very large.

Brett

[waxmiguel]

yes

>> *Contact*
>> abe...@chromium.org <abe...@chromium.org>
>>
>> *Summary*

>>
>> The Chrome PAK files includes inlined versions of a large numbers of
>> HTML,
>> CSS, and Javascript files. Many of these files contain extensive comments
>> and whitespace, hence increasing the download and on disk size all
>> versions
>> of Chrome for no benefit. This project will implement changes to the
>> build
>> system to strip whitespace and comments from these files.
>>

>> *Tracking bug*
>> https://crbug/619091
>>
>> *Design doc*

>>
>> https://docs.google.com/document/d/1XUte4m_wkPwQ9I-MfVZfBhksoL0E2_7Eb7N-PkXQKvw/edit?usp=sharing
>>
>>
>>
>> --
>> --
>> Chromium Developers mailing list: chromi...@chromium.org
>> View archives, change email options, or unsubscribe:
>> http://groups.google.com/a/chromium.org/group/chromium-dev
>>
>
> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev

> ---
> You received this message because you are subscribed to the Google Groups
> "Chromium-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-dev...@chromium.org.
>

--
waxmi...@gmail.com
waxm...@mail.ru
waxm...@bitrix24.com
0xC840F256B0F0FF9069E918d060063057AAaA6b36

[Anthony Berent]

Brett,

Doing only on official builds is certainly an option, but given the limited testing we do on official builds I tend to dislike anything that increases the difference between them and development builds (although this should make absolutely no difference to the behaviour of Chrome, it would be nice to catch any instances where it does early).

While I think I understand why you and others are worried by this, one thing I do not understand is how this is different from the situation with C++ header files, which are all listed both in the GN files and the source code, with no checks I know of that the lists in the GN files are complete.

[Nico Weber]

On Tue, Jul 12, 2016 at 2:36 PM, Anthony Berent <abe...@chromium.org> wrote:

Brett,

Doing only on official builds is certainly an option, but given the limited testing we do on official builds I tend to dislike anything that increases the difference between them and development builds (although this should make absolutely no difference to the behaviour of Chrome, it would be nice to catch any instances where it does early).

While I think I understand why you and others are worried by this, one thing I do not understand is how this is different from the situation with C++ header files

The difference is that forgetting to list a .h file in a GN file doesn't change the result of the build, it only means the MSVC/Xcode projects that gn generates won't list these .h files.

[Anthony Berent]

On Tue, 12 Jul 2016 at 19:53 Nico Weber <tha...@chromium.org> wrote:

On Tue, Jul 12, 2016 at 2:36 PM, Anthony Berent <abe...@chromium.org> wrote:

Brett,

Doing only on official builds is certainly an option, but given the limited testing we do on official builds I tend to dislike anything that increases the difference between them and development builds (although this should make absolutely no difference to the behaviour of Chrome, it would be nice to catch any instances where it does early).

While I think I understand why you and others are worried by this, one thing I do not understand is how this is different from the situation with C++ header files

The difference is that forgetting to list a .h file in a GN file doesn't change the result of the build, it only means the MSVC/Xcode projects that gn generates won't list these .h files.

Doesn't it also mean that changes to that .h file will not cause that build step to be repeated, so causing incremental builds to be potentially wrong?  

[Nico Weber]

On Tue, Jul 12, 2016 at 2:58 PM, Anthony Berent <abe...@chromium.org> wrote:

On Tue, 12 Jul 2016 at 19:53 Nico Weber <tha...@chromium.org> wrote:

On Tue, Jul 12, 2016 at 2:36 PM, Anthony Berent <abe...@chromium.org> wrote:

Brett,

Doing only on official builds is certainly an option, but given the limited testing we do on official builds I tend to dislike anything that increases the difference between them and development builds (although this should make absolutely no difference to the behaviour of Chrome, it would be nice to catch any instances where it does early).

While I think I understand why you and others are worried by this, one thing I do not understand is how this is different from the situation with C++ header files

The difference is that forgetting to list a .h file in a GN file doesn't change the result of the build, it only means the MSVC/Xcode projects that gn generates won't list these .h files.

Doesn't it also mean that changes to that .h file will not cause that build step to be repeated, so causing incremental builds to be potentially wrong?  

No, ninja gets .h dependency information from the compiler, not from gn.

[Nico Weber]

On Tue, Jul 12, 2016 at 3:04 PM, Nico Weber <tha...@chromium.org> wrote:

On Tue, Jul 12, 2016 at 2:58 PM, Anthony Berent <abe...@chromium.org> wrote:

On Tue, 12 Jul 2016 at 19:53 Nico Weber <tha...@chromium.org> wrote:

On Tue, Jul 12, 2016 at 2:36 PM, Anthony Berent <abe...@chromium.org> wrote:

Brett,

Doing only on official builds is certainly an option, but given the limited testing we do on official builds I tend to dislike anything that increases the difference between them and development builds (although this should make absolutely no difference to the behaviour of Chrome, it would be nice to catch any instances where it does early).

While I think I understand why you and others are worried by this, one thing I do not understand is how this is different from the situation with C++ header files

The difference is that forgetting to list a .h file in a GN file doesn't change the result of the build, it only means the MSVC/Xcode projects that gn generates won't list these .h files.

Doesn't it also mean that changes to that .h file will not cause that build step to be repeated, so causing incremental builds to be potentially wrong?  

No, ninja gets .h dependency information from the compiler, not from gn.

(Actions can do something similar -- see `gn help depfile`. There are a few examples in the codebase.)

[Anthony Berent]

Ok, I will take a look at how that works. The ideal would be take the output of 'grit buildinfo', edit it, and feed it back into the build system, however my I didn't see how this was possible when I looked before. I will take another look in the morning (London time).

Meanwhile if anybody knows of any examples of doing anything of the sort with GN, please send me pointers.

[Brett Wilson]

On Tue, Jul 12, 2016 at 12:11 PM, Anthony Berent <abe...@chromium.org> wrote:

Ok, I will take a look at how that works. The ideal would be take the output of 'grit buildinfo', edit it, and feed it back into the build system, however my I didn't see how this was possible when I looked before. I will take another look in the morning (London time).

Meanwhile if anybody knows of any examples of doing anything of the sort with GN, please send me pointers.

Grit already writes a .d file for proper dependency tracking, which is why the sources don't need to be written in the build files in the first place but we still get proper incremental rebuilds. Be aware that we don't want to shell out to grit at GN-time to get stuff out of it. I actually spent a long time fixing grit to not require this (it used to) since this basically doubled GN's runtime on Windows. 

Brett

[Anthony Berent]

It seems to me that we have three choices:

• We can compile all the Javascript files every time we run grit. As discussed earlier in this thread this will significantly slow down the builds.
• We can list the Javascript files in the BUILD.gn files. This has the problem that the files are listed twice, once in the BUILD.gn files and once in grit's inputs.
• We can generate a list of files to be compiled at GN time. This, as I explain below, requires running grit buildinfo, or something equivalent, at GN time, so will significantly slow down the GN step.

I think it is logically impossible to correctly use a dependency file from a previous build, or anything similar for this, to generate the build steps. This obviously won't be possible in a clean build; but consider also the case when a single new Javascript file is added. In this case, if we try to generate the steps from the dependency file we will miss the new file, so the first build after a file is added will not compress it.

(This is different from, for example, a C++ compilation step, where the dependency file is associated with a single compilation step, and simply controls whether that step is run, not what steps need to be generated).

Given this I think the only way of generating the compile steps is to run grit buildinfo. I don't think that this can be done at build time, since I don't think ninja supports creating additional build steps during a build; so I think it has to be done at GN time.

Please point out any flaws in my logic, or any options I have missed.

Brett

[Nico Weber]

On Wed, Jul 13, 2016 at 8:45 AM, Anthony Berent <abe...@chromium.org> wrote:

On Tue, 12 Jul 2016 at 20:40 Brett Wilson <bre...@chromium.org> wrote:

On Tue, Jul 12, 2016 at 12:11 PM, Anthony Berent <abe...@chromium.org> wrote:

Ok, I will take a look at how that works. The ideal would be take the output of 'grit buildinfo', edit it, and feed it back into the build system, however my I didn't see how this was possible when I looked before. I will take another look in the morning (London time).

Meanwhile if anybody knows of any examples of doing anything of the sort with GN, please send me pointers.

Grit already writes a .d file for proper dependency tracking, which is why the sources don't need to be written in the build files in the first place but we still get proper incremental rebuilds. Be aware that we don't want to shell out to grit at GN-time to get stuff out of it. I actually spent a long time fixing grit to not require this (it used to) since this basically doubled GN's runtime on Windows. 

It seems to me that we have three choices:

• We can compile all the Javascript files every time we run grit. As discussed earlier in this thread this will significantly slow down the builds.
• We can list the Javascript files in the BUILD.gn files. This has the problem that the files are listed twice, once in the BUILD.gn files and once in grit's inputs.
• We can generate a list of files to be compiled at GN time. This, as I explain below, requires running grit buildinfo, or something equivalent, at GN time, so will significantly slow down the GN step.

I think 1 and 3 are out from a build performance point of view. 2 has the problem you mention, so let's try to fix that. Ideas:

* List files in gn, and pass them to grit on the command line (or in a response file) and don't list them in the .grd file

* List them in both places, but give grit a thingy so it can check that the listed js files are equal to a list of files passed on the command line. Still listed twice, but this way they can't diverge (…now that I wrote this, I think we do something similar for grit's <output> elements already, or something in that area? I think I remember reviewing a CL from Brett in that area a few years ago)

[Anthony Berent]

On Wed, 13 Jul 2016 at 16:35 Nico Weber <tha...@chromium.org> wrote:

On Wed, Jul 13, 2016 at 8:45 AM, Anthony Berent <abe...@chromium.org> wrote:

On Tue, 12 Jul 2016 at 20:40 Brett Wilson <bre...@chromium.org> wrote:

On Tue, Jul 12, 2016 at 12:11 PM, Anthony Berent <abe...@chromium.org> wrote:

Ok, I will take a look at how that works. The ideal would be take the output of 'grit buildinfo', edit it, and feed it back into the build system, however my I didn't see how this was possible when I looked before. I will take another look in the morning (London time).

Meanwhile if anybody knows of any examples of doing anything of the sort with GN, please send me pointers.

Grit already writes a .d file for proper dependency tracking, which is why the sources don't need to be written in the build files in the first place but we still get proper incremental rebuilds. Be aware that we don't want to shell out to grit at GN-time to get stuff out of it. I actually spent a long time fixing grit to not require this (it used to) since this basically doubled GN's runtime on Windows. 

It seems to me that we have three choices:

• We can compile all the Javascript files every time we run grit. As discussed earlier in this thread this will significantly slow down the builds.
• We can list the Javascript files in the BUILD.gn files. This has the problem that the files are listed twice, once in the BUILD.gn files and once in grit's inputs.
• We can generate a list of files to be compiled at GN time. This, as I explain below, requires running grit buildinfo, or something equivalent, at GN time, so will significantly slow down the GN step.

I think 1 and 3 are out from a build performance point of view. 2 has the problem you mention, so let's try to fix that. Ideas:

* List files in gn, and pass them to grit on the command line (or in a response file) and don't list them in the .grd file

Grit need more information about the files than is in, or, I think, can sensibly be added to, gn. There are two cases, but in both cases grit needs more information than just the file name. For javascript files that are directly referenced in the.grd file the .grd file maps them to a resource id. Many (most, I think) of the javascript files, however, are indirectly referenced through <script..> elements in HTML files For those files the place, or places, where they are referenced is important.

* List them in both places, but give grit a thingy so it can check that the listed js files are equal to a list of files passed on the command line. Still listed twice, but this way they can't diverge (…now that I wrote this, I think we do something similar for grit's <output> elements already, or something in that area? I think I remember reviewing a CL from Brett in that area a few years ago)

Yes. This is the best (or possibly the least bad) solution I know of. I originally saw this as a possible, low priority, future enhancement; but given how worried people are about the maintenance  issue I will add it to my current CL (unless someone comes up with a better solution in the next few hours).

[Anthony Berent]

Further discussion of this has happened on the CL (https://codereview.chromium.org/2094193004/, see in particular brettw@'s comment and my reply), and I am now going to at least try out doing this from grit, but only doing it in official builds.

[Anthony Berent]

The new CL is now out for review (https://codereview.chromium.org/2179033002/). Rather to my surprise ninja parallelizes this well, so it doesn't seem to significantly slow down builds. As such I have enabled this for all Android builds.