Closed Bug 1382511 Opened 3 years ago Closed 3 years ago

Consider adding --output-sync=line to our make invocation

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.
59 bytes, text/x-review-board-request
glandium
: review+
Details 
I was using a script to scrape build logs and compare command lines and found a lot of uninteresting differences between builds due to interleaved output between make processes. Perhaps a little surprisingly, adding `MAKEFLAGS += --output-sync=line` fixes these differences. It may be worth adding this in general.
See previous discussion around --output-sync in bug 1285254.
Depends on: 1285254
(In reply to Gregory Szorc [:gps] from comment #1)
> See previous discussion around --output-sync in bug 1285254.

The issue there seems to be interleaving between multiple make invocations, which I don't think this would solve. This appears to address interleaving between worker processes spawned by make.
Actually, --output-sync=line might fix both issues.
Ok, well, let's see.
Assignee: nobody → cmanchester
Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

https://reviewboard.mozilla.org/r/159432/#review165220

Consider make 4.2 is out, at some point we'll probably want to add make version sniffing to configure and set a config variable for whether --output-sync is supported.

I'm not telling you to scope bloat this patch. But it would be a nice addition!
Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

https://reviewboard.mozilla.org/r/159432/#review166082

::: client.mk:78
(Diff revision 1)
>  ifdef CONFIG_GUESS_SCRIPT
>    CONFIG_GUESS := $(shell $(CONFIG_GUESS_SCRIPT))
>  endif
>  
> +ifeq (4.0,$(firstword $(sort 4.0 $(MAKE_VERSION))))
> +MAKEFLAGS += --output-sync=line

Should probably be MOZ_MAKE_FLAGS
Attachment #8888480 - Flags: review?(mh+mozilla)
Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

https://reviewboard.mozilla.org/r/159432/#review168652
Attachment #8888480 - Flags: review?(mh+mozilla) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae6577d57698
Add --output-sync=line to MOZ_MAKE_FLAGS where available. r=glandium
Depends on: 1386739
as a note the compiler warnings are increased with this change:
== Change summary for alert #8470 (as of August 01 2017 18:22 UTC) ==

Regressions:

  1%  compiler warnings summary linux64 asan asan debug     11,657.33 -> 11,809.00
  1%  compiler warnings summary linux64 asan asan opt       9,491.58 -> 9,561.08

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8470
(In reply to Joel Maher ( :jmaher) (UTC-9) from comment #12)
> as a note the compiler warnings are increased with this change:
> == Change summary for alert #8470 (as of August 01 2017 18:22 UTC) ==
> 
> Regressions:
> 
>   1%  compiler warnings summary linux64 asan asan debug     11,657.33 ->
> 11,809.00
>   1%  compiler warnings summary linux64 asan asan opt       9,491.58 ->
> 9,561.08
> 
> For up to date results, see:
> https://treeherder.mozilla.org/perf.html#/alerts?id=8470

The warnings database checks each line of build output against a regex, so I guess we were missing some before due to interleaving.
Yes, historically the compiler warnings parser has been sensitive to interleaving.

I can't remember if it attempts to parse multiple lines, but some compiler warning output spans multiple lines and interleaved output that isn't synchronized at the process level (in terms of a make recipe's view of a process), then things get weird.
https://hg.mozilla.org/mozilla-central/rev/ae6577d57698
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.