Closed Bug 1557788 Opened 2 years ago Closed 2 years ago

Remove dead PGO code

Categories

(Firefox Build System :: General, task)

task
Not set
normal

Tracking

(firefox73 fixed)

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(8 files)

Once all platforms are using the 3-tier PGO model, we'll have some dead PGO code paths. Some examples are:

  • The 'profiledbuild' target in $topsrcdir/Makefile.in
  • Probably the 'pgo_build' config and _compile_against_pgo() in mozharness

One thing that should be addressed beforehand is to make sure that local developers who want to build with PGO still have usable workflows. For example, are all 3 parts necessary for doing local PGO tests? Or would it be feasible to simply download a profile from an existing job and build only the 3rd tier?

We should be able to do both, which would actually be an improvement compared to now.

In general I'm not opposed to the removal of 1-tier PGO, but I'd like to ask that, whatever happens here, we still have an easy way to do local PGO builds that match the settings of automation. Aside from a small handful of build peers, most people who do PGO builds only do them rarely, so it's very easy to forget a detail (or not know you need it in the first place). Specifically I'm trying to avoid potential footguns like, you dug up a six-month old PGO mozconfig, and for some reason you didn't get LTO in the new scheme, and didn't notice until days of debugging later.

Depends on: 1561144
Depends on: 1561147
Depends on: 1563402
Depends on: 1563403
Assignee: nobody → mshal

xvfb was used to create a virtual framebuffer for running Firefox during
build jobs to support PGO profile generation. That now runs in a
separate task, so we don't need this flag for builds anymore.

Note that other Linux builds still need xvfb in order to run xpcshell in
'make check'.

MOZ_1TIER_PGO was a temporary hack to support 1-tier PGO builds while
they were being ported to 3-tier. Now that all builds are 3-tier, it can
be removed.

Depends on D56111

pgomerge.py was needed for Windows MSVC PGO builds. Now that we use
clang for these builds, it's no longer used.

Depends on D56112

This was used for Windows MSVC PGO builds that re-used the objdir. We
don't use MSVC for PGO anymore, and we don't re-use the objdir.

Depends on D56113

In 1-tier PGO builds that shared the objdir between the instrumented and
profile-use builds, the instrumented build objects used a different
suffix (.i_o) to separate them from the profile-use build (which uses
the default .o suffix). These builds are now always in separate objdirs,
and don't need special suffix rules anymore.

As a bonus, this helps fix an issue with buildid.cpp continually
rebuilding because libxul_so.list always lists the inputs as *.o, which
don't exist if we're using a .i_o suffix. Make would always re-create
buildid.cpp and therefore libxul.so in the instrumented build even when
nothing has changed.

Depends on D56114

One of these appears to be mistakenly leftover from bug 861178
which was intended to avoid purging dist/, but the code inside the ifndef
no longer does that.

The other is from bug 1246881 to avoid re-writing the buildid.h file
during the profile-use build, but now that there are two separate
builds, they will each have their own buildid.h / source-repo.h files.

Depends on D56115

MOZ_PGO_PROFILE_USE is used to enable certain features in the automation
mozconfigs for profile-use builds, including the MOZ_PROFILE_USE
configure option. They aren't easily combined into one flag due to the
extra settings that are enabled only in automation mozconfigs.

Renaming the former variable to TASKCLUSTER_PGO_PROFILE_USE makes it
slightly more obvious where it comes from and avoids confusion.

Depends on D56116

This is no longer used as of bug 1553065.

Depends on D56117

Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/95d72170f969
Remove NEED_XVFB from Linux instrumented and rusttest builds; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/15921c0ba6c8
Remove MOZ_1TIER_PGO; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/9f837b4fe264
Remove pgomerge.py; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/3650f8bc5e2a
Remove pgo.relink touches; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/7dea0dd6a868
Remove OBJS_VAR_SUFFIX & .i_o suffix for instrumented builds; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/c41a9af98fbe
Remove spurious MOZ_PROFILE_USE ifndefs; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/fd2851b8f09c
Rename MOZ_PGO_PROFILE_USE to TASKCLUSTER_PGO_PROFILE_USE; r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/c369fd660c23
Remove unused mozharness build variant; r=firefox-build-system-reviewers,chmanchester

\o/ Thanks mshal!

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