Closed Bug 758354 Opened 11 years ago Closed 11 years ago
Stop preprocessing chrome unnecessarily
1.06 KB, text/plain
7.13 KB, text/plain
121.15 KB, patch
|Details | Diff | Splinter Review|
30.43 KB, text/plain
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.
Seems like we could also make the preprocessor emit a warning when it ends up doing nothing.
I excluded 222 DOM Inspector files because of its funky locales setup, but I suspect that none of its locales need to be preprocessed ;-)
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
See Also: → 760126
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #636116 - Flags: review?(dolske)
Attachment #636116 - Flags: review?(dolske) → review+
(In reply to :Gavin Sharp (use email@example.com 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.
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 on attachment 636116 [details] [diff] [review] Proposed patch Pushed mozilla-inbound changeset ec9451e9e830.
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
Pushed integration/mozilla-inbound changeset 1718e78e0cb0.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Comment on attachment 636173 [details] Sample output Not sure what feedback you're looking for here, but sure.
Attachment #636173 - Flags: feedback?(dolske) → feedback+
(followup bug for any such build changes though, please)
Comment on attachment 636173 [details] Sample output yes, let's fix these!?
You need to log in before you can comment on or make changes to this bug.