Closed Bug 1522066 Opened 9 months ago Closed 9 months ago

Prevent unused files in Intl/ICU from being built

Categories

(Firefox Build System :: General, enhancement, major)

enhancement
Not set
major

Tracking

(firefox67 fixed)

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: decoder, Assigned: decoder)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently, the Intl library is fully built despite the fact that we only use a fraction of it. We should disable the unused parts by not including those files in the sources.mozbuild generation. Not only does this save build time and reduce code size, but it also makes our work in code coverage analysis easier.

Patching coming.

The attached patch removes all files detected as dead by the Reachable project, which includes 84 C++ files or 36600 lines of C++ code. An earlier Try push on all platforms was green \o/

Does it look safe to remove?

Flags: needinfo?(m_kato)
Flags: needinfo?(jwalden)
Flags: needinfo?(andrebargull)

I am OK. We may have to update this list when updating ICU.

Flags: needinfo?(m_kato)

So this is a manual update? What's the process for regenerating it? Where is it documented? At absolute minimum (and possibly this is a perfectly adequate amount), there ought be a comment next to the unused-sources list indicating how to generate it.

How easy is it for a random developer to run it himself? ICU updates are not frequent, but they are a regular occurrence. And occasionally we need to make use of extra ICU APIs in the current imported code, so we'd have to trim the unused-sources list. How easy is it for someone who doesn't know anything about any of this to do so?

(In reply to Jeff Walden [:Waldo] from comment #5)

So this is a manual update? What's the process for regenerating it? Where is it documented? At absolute minimum (and possibly this is a perfectly adequate amount), there ought be a comment next to the unused-sources list indicating how to generate it.

It is currently not possible to fully generate this automatically and the tool used to compile this list isn't public yet. I can add a comment to the list to contact me in case of any problems or to rebuild the list.

And occasionally we need to make use of extra ICU APIs in the current imported code, so we'd have to trim the unused-sources list. How easy is it for someone who doesn't know anything about any of this to do so?

For most of the files added to this list, simply removing the file containing the desired API from the list is sufficient. There are only a few larger clusters in the list that can only be re-enabled altogether. I could format the list in such a way that the clusters are visible, so any developer could remove the files as a whole.

Pushed by choller@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/307982b99756
Disable unused ICU code during build. r=froydnj,m_kato

Landed this with additional comments and the clustering as outlined in comment 6. This should make it trivial for developers to re-enable parts if ICU if they need them.

Flags: needinfo?(jwalden)
Flags: needinfo?(andrebargull)
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Out of curiosity, why are the excluded source files, the ones listed in UNUSED_SOURCES, kept in the tree?

Because it is easier to sync with new upstream releases, especially if these files become useful.
And the cost of keeping them in tree is tiny.

You need to log in before you can comment on or make changes to this bug.