758354 - Stop preprocessing chrome unnecessarily

Status: RESOLVED FIXED

(Toolkit Graveyard :: Build Config, defect)

Description

[[e-mail]]

Attachment: Shell script to detect unpreprocessed files

I wrote a shell script to see which files were being preprocessed unnecessarily. Within toolkit it found 55 preprocessed files that lacked any preprocessor directives. It increased to 128 when you included files that lacked preprocessor directives that started with a a letter, which I assume to mean that the file only contains a preprocessed licence block. Now that these are only three lines long, they are probably no longer worth preprocessing.

The script's first parameter is either '' or '\w' depending on whether you're looking for actual preprocessor statements. (The ^# or ^% is added by the script.) The remaining parameters, if any, are the directories to search.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Seems like we could also make the preprocessor emit a warning when it ends up doing nothing.

Comment 2

[[e-mail]]

Attachment: List of all preprocessless files in comm-central and dependencies

I excluded 222 DOM Inspector files because of its funky locales setup, but I suspect that none of its locales need to be preprocessed ;-)

Comment 3

[Joshua Cranmer [:jcranmer]]

I have a script that automatically fixes useless preprocessing (including both ones that do nothing and ones that just use it for stripping out comments) attached; it can be found on bug 760126. I disclaim any and all problems that could come from using the script :-P

Comment 4

[[e-mail]]

Attachment: Proposed patch

Comment 5

[Justin Dolske [:Dolske]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #1)
> Seems like we could also make the preprocessor emit a warning when it ends
> up doing nothing.

I really like this idea.

Comment 6

[[e-mail]]

Attachment: Sample output

So, this is edited output from a depend rebuild of SeaMonkey using a locally hacked Preprocessor and JarMaker. There are three sources of mispreprocessing:
1. Unnecessarily preprocessed chrome
2. Unnecessarily preprocessed modules or components
3. Duplicate preprocessing
application.ini and update-settings.ini appears to fall in category 3.

Comment 7

[[e-mail]]

Comment on attachment 636116 [details] [diff] [review]
Proposed patch

Pushed mozilla-inbound changeset ec9451e9e830.

Comment 8

[Ed Morley [:emorley]]

Sorry, had to back this out for PGO permaorange on linux:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ec9451e9e830&jobname=pgo%20test%20mochitest-other

eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=12968419&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/a05fc1998f5e

Comment 9

[[e-mail]]

Pushed integration/mozilla-inbound changeset 1718e78e0cb0.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1718e78e0cb0

Comment 11

[Justin Dolske [:Dolske]]

Comment on attachment 636173 [details]
Sample output

Not sure what feedback you're looking for here, but sure.

Comment 12

[Justin Dolske [:Dolske]]

(followup bug for any such build changes though, please)

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 636173 [details]
Sample output

yes, let's fix these!?