Closed
Bug 1483394
Opened 6 years ago
Closed 6 years ago
Remove unneeded #includes of nsContentUtils.h in /layout
Categories
(Core :: Layout, enhancement, P4)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(2 files)
I watched an intern presentation today about finding the files in mozilla-central for which changes are the most expensive. (where "expensive" = $DOWNSTREAM-FILES x $CHANGE-FREQUENCY) nsContentUtils.h was towards the top. As it happens, we have lots of includes of this file which are probably unneeded (based on the rough heuristic that the #include line for "nsContentUtils.h" is the only usage of the word nsContentUtils in the file). Let's get rid of these likely-unnecessary #includes where possible, to reduce $DOWNSTREAM-FILES and make incremental builds faster when nsContentUtils.h changes (as it does frequently).
Assignee | ||
Comment 1•6 years ago
|
||
Here's the search I'm using to get my list of candidate files: https://searchfox.org/mozilla-central/search?q=nsContentUtils&path=layout%2F I'm skimming that list for entries where #include is the only thing. (And I'm also doing a sanity-check for "scriptblocker" when editing the files, because nsAutoScriptBlocker is probably the most commonly-used thing that is *not* in the nsContentUtils namespace which is exported by nsContentUtils.h.)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•6 years ago
|
||
For each file touched in this patch, thee file had an #include for nsContentUtils.h, but no other mentions of the string "nsContentUtils", nor any mention of its "ScriptBlocker"-related types. So these files likely don't need their nsContentUtils.h include anymore, and we can remove it to get a marginal win on build time/complexity.
Assignee | ||
Updated•6 years ago
|
Priority: -- → P4
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #0) > I watched an intern presentation today about finding the files in > mozilla-central for which changes are the most expensive. (FWIW, this project is tracked in bug 1474442 & related bugs. I'm attaching a screenshot of the slide with the "most expensive files" as judged by this tool/analysis. nsContentUtils.h is the 5th-most-expensive file to change, according to this ranking.)
Comment 4•6 years ago
|
||
Comment on attachment 9000112 [details] Bug 1483394: Remove unneeded #includes of nsContentUtils.h in /layout. r?TYLin Ting-Yu Lin [:TYLin] (UTC-7) has approved the revision.
Attachment #9000112 -
Flags: review+
Assignee | ||
Comment 5•6 years ago
|
||
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b93f30ff4472917f92ad9e706dc554df3fa2edd0 The reds there seem unrelated. Gonna go ahead and land.
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/68faa8b52f60 Remove unneeded #includes of nsContentUtils.h in /layout. r=TYLin
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/68faa8b52f60
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•