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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Andrew Halberstadt [:ahal]]

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

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: 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.

Comment 2

[Andrew Halberstadt [:ahal]]

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.

Comment 3

[Mike Hommey [:glandium]]

I'm tempted to say we should remove the footer thing.

Comment 4

[Andrew Halberstadt [:ahal]]

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 5

[Gregory Szorc [:gps]]

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.

Comment 6

[Andrew Halberstadt [:ahal]]

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..

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/93afc0ae4046

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/93afc0ae4046