Closed Bug 1194415 Opened 9 years ago Closed 9 years ago

Refactor BuildProgressFooter.draw() for |mach build| perf improvements

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox43 fixed)

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I was doing some profiling for bug 1027665 and noticed that over a ~19 min build on my Fedora machine, we were spending around 45s drawing the TIER status in the terminal.

A good 10s of that was spent querying the state of the TIERs over and over again [1]. The refactoring below replaces the querying with a dictview. Along with some other minor speedups, the change results in only 29s spent drawing the footer.

[1] https://dxr.mozilla.org/mozilla-central/rev/7649ffe28b67aa2dad0f67ea01500c0ff91b2bac/python/mozbuild/mozbuild/mach_commands.py#153
Bug 1194415 - Refactor BuildProgressFooter.draw() for minor |mach build| perf improvement, r=gps

Over a 19 minute build, |mach build| was spending ~45s drawing the TIER footer. This brings it down to ~30s.
Attachment #8647686 - Flags: review?(gps)
Comment on attachment 8647686 [details]
MozReview Request: Bug 1194415 - Refactor BuildProgressFooter.draw() for minor |mach build| perf improvement, r=gps

Bug 1194415 - Refactor BuildProgressFooter.draw() for minor |mach build| perf improvement, r=gps

Over a 19 minute build, |mach build| was spending ~45s drawing the TIER footer. This brings it down to ~30s.
Blocks: 1027665
I'm tempted to say we should remove the footer thing.
I should clarify that those are the cumulative times we spend in that function. The actual build happens in a subprocess, so I'm not entirely sure how it impacts end to end time.
Comment on attachment 8647686 [details]
MozReview Request: Bug 1194415 - Refactor BuildProgressFooter.draw() for minor |mach build| perf improvement, r=gps

https://reviewboard.mozilla.org/r/16031/#review14633

This looks good.

I agree with glandium that the footer is providing marginal value these days. I added it initially so people (especially build system developers) could see where time was being spent and would have some notion of build progress. I would like to continue having a notion of progress (and estimated completion someday). But I don't think the footer in its current form is doing that much and could be removed if we prove it is actually slowing down builds. Theoretically it can, as make will stall if its stdout buffer can't be written to (blocking I/O) and I'm pretty sure mozprocess waits on on_line callbacks to fire before consuming more stdout from make. (Well, it has to because of the damn GIL.) But Python should release the GIL when doing I/O to the terminal. So if mozprocess has a background thread reading from make's stdout and the stdout buffer can never be full, we shouldn't see a wall time increase.
Attachment #8647686 - Flags: review?(gps) → review+
Yeah, that's what I thought too, and I'm pretty sure that mozprocess bug got fixed:
https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozprocess/mozprocess/processhandler.py?offset=700#858

Maybe it's not working as we expect though..
https://hg.mozilla.org/mozilla-central/rev/93afc0ae4046
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: