Single-machine PGO builds don't get NS_FREE_PERMANENT_DATA
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox69 fixed)
| Tracking | Status | |
|---|---|---|
| firefox69 | --- | fixed |
People
(Reporter: away, Assigned: Gijs)
References
Details
Attachments
(1 file)
I haven't confirmed in the output, but by inspection, 4b78f4fb473c doesn't do the right thing on single-machine PGO builds. In those builds, MOZ_PROFILE_GENERATE is set by the profiledbuild:: target rather than --enable-profile-... switches.
An additional trickiness is that single-machine builds only set the C++ define in some isolated directories which may not play nice with the fact that nscore.h is included from a bunch of different places.
|
Assignee | |
Comment 1•5 years ago
•
|
||
Is the MOZ_PROFILE_GENERATE config flag going to be already set in that toolchain.configure file (as a result of the profiledbuild target), ie can we also make that trigger the define in the same toolchain.configure file? Or does that not work?
I'm afraid I don't speak enough build to be able to answer that.
|
||
Comment 3•5 years ago
|
||
FWIW I am close to getting Windows switched over to 3-tier PGO (perhaps before the all-hands?), so depending on your timeline you may be able to wait for that. I just filed bug 1557785 if you want to follow along. The profiledbuild version just sets a make variable, and unfortunately doesn't go through configure.
|
||
Comment 4•5 years ago
•
|
||
(In reply to Michael Shal [:mshal] from comment #3)
FWIW I am close to getting Windows switched over to 3-tier PGO (perhaps before the all-hands?), so depending on your timeline you may be able to wait for that. I just filed bug 1557785 if you want to follow along. The profiledbuild version just sets a make variable, and unfortunately doesn't go through configure.
Right... configure and mozbuild are basically unaware of MOZ_PROFILE_GENERATE in single-machine pgo builds. If we need to set it that probably needs to happen around here: https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/config/config.mk#135
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 5•5 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #3)
FWIW I am close to getting Windows switched over to 3-tier PGO (perhaps before the all-hands?), so depending on your timeline you may be able to wait for that. I just filed bug 1557785 if you want to follow along. The profiledbuild version just sets a make variable, and unfortunately doesn't go through configure.
I think our only real timeline is "before 69 merges to beta". There might have been more schedule pressure if there was a perf regression, but I think so far nothing's come up... Does that sound safe?
(In reply to Chris Manchester (:chmanchester) from comment #4)
Right... configure and mozbuild are basically unaware of MOZ_PROFILE_GENERATE in single-machine pgo builds. If we need to set it that probably needs to happen around here: https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/config/config.mk#135
FWIW, I tried to look at this just to make sure we have a failsafe if for some reason we can't switch to the 3-tier thing, but it's not obvious to me how we'd do this, presumably because I've forgotten any make-speak I may have once known. Would we have to string-modify the list of defines in here somehow, or is there an easier way? Because that feels pretty yucky...
|
|
||
Comment 6•5 years ago
|
||
Something like PGO_CFLAGS += -DNS_FREE_PERMANENT_DATA=1 in that block I linked should do it (yes it's yucky).
|
|
Assignee | |
Comment 7•5 years ago
|
||
|
|
Assignee | |
Comment 8•5 years ago
|
||
(In reply to Chris Manchester (:chmanchester) from comment #6)
Something like
PGO_CFLAGS += -DNS_FREE_PERMANENT_DATA=1in that block I linked should do it (yes it's yucky).
Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1559878#c3 I think this works, so I'll just attach a patch for this...
|
|
||
Comment 9•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #5)
(In reply to Michael Shal [:mshal] from comment #3)
FWIW I am close to getting Windows switched over to 3-tier PGO (perhaps before the all-hands?), so depending on your timeline you may be able to wait for that. I just filed bug 1557785 if you want to follow along. The profiledbuild version just sets a make variable, and unfortunately doesn't go through configure.
I think our only real timeline is "before 69 merges to beta". There might have been more schedule pressure if there was a perf regression, but I think so far nothing's come up... Does that sound safe?
Sorry for the delay, I was hoping to report that the 3-tier PGO would've landed sooner (should be this week, I think?)
In any case, I don't think there are any blockers that will prevent it from going forward. Your patch should work fine in the meantime though, and we can always pull it back out as part of the final PGO cleanup for simplicity (bug 1557788).
|
||
Comment 10•5 years ago
|
||
Gijs, can you land your patch? It looks like it's blocking enabling the socket process in bug 1558205.
|
|
Assignee | |
Comment 11•5 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #10)
Gijs, can you land your patch? It looks like it's blocking enabling the socket process in bug 1558205.
Sorry, I thought I had, looks like bad wifi interfered with me using lando or something. Tree's closed right now but I've submitted it.
|
|
Assignee | |
Comment 12•5 years ago
|
||
(Though, bug 1557785 is on central now, so I think we might be OK? Hard to tell from bug 1558205.)
|
||
Comment 13•5 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/455dff329fcc ensure we define NS_FREE_PERMANENT_DATA for single-stage pgo builds, r=chmanchester
|
|
||
Comment 14•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #12)
(Though, bug 1557785 is on central now, so I think we might be OK? Hard to tell from bug 1558205.)
I'm landing the last of the patches to move all central builds to 3-tier PGO now, but I think your patch is still good to land in case I've missed anything. (In particular, I'm not certain all beta / release builds are using 3-tier builds yet).
|
||
Comment 15•5 years ago
|
||
Backed out for build bustages
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=455dff329fcc9d8fa0647ca662ce5c8451004c87&selectedJob=253803120
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253803120&repo=autoland&lineNumber=4710
Backout: https://hg.mozilla.org/integration/autoland/rev/40bcaacdcf614e325f0538754a99b4c99411d26f
|
|
Assignee | |
Comment 16•5 years ago
|
||
|
|
||
Comment 17•5 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4cb7eb5b8a42 ensure we define NS_FREE_PERMANENT_DATA for single-stage pgo builds, r=chmanchester
|
||
Comment 18•5 years ago
|
||
| bugherder | ||
|
||
Comment 19•5 years ago
|
||
== Change summary for alert #21700 (as of Tue, 02 Jul 2019 07:55:00 GMT) ==
Improvements:
4% raptor-speedometer-firefox windows7-32-shippable opt 77.58 -> 80.83
4% raptor-speedometer-firefox windows7-32-shippable opt 77.60 -> 80.69
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=21700
Description
•