Closed Bug 1541068 Opened 2 years ago Closed 2 years ago

MOZ_PGO: `maybe_clobber_profiledbuild` step does not delete Rust artefacts.

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: mwoerister, Assigned: froydnj)

References

Details

Attachments

(2 files)

Attached file ???

It seems that the maybe_clobber_profiledbuild step does not delete Rust artefacts.

Flags: needinfo?(nfroyd)

This is because we never added rust directories to GARBAGE_DIRS, either when we had Rust compilation scattered about the tree, or when we moved everything to a single toplevel workspace. Should be an easy fix?

Assignee: nobody → nfroyd
Flags: needinfo?(nfroyd)
Blocks: 1437452

We're tentatively on track move remaining PGO builds to 3-tier PGO soon. That would probably render this issue obsolete.

(In reply to Chris Manchester (:chmanchester) from comment #2)

We're tentatively on track move remaining PGO builds to 3-tier PGO soon. That would probably render this issue obsolete.

This would affect "shippable" builds (or really any build that does PGO, whether or not it has that in its name).

We're tentatively on track move remaining PGO builds to 3-tier PGO soon. That would probably render this issue obsolete.

Can you tell me more about this? It's unclear to me how this interacts with planned PGO support in the Rust compiler.

(In reply to Michael Woerister from comment #4)

We're tentatively on track move remaining PGO builds to 3-tier PGO soon. That would probably render this issue obsolete.

Can you tell me more about this? It's unclear to me how this interacts with planned PGO support in the Rust compiler.

The idea behind 3-tier/stage PGO (bug 1507330) is that we want to perform the instrumented build, the actual profiling, and the profile-use build as three separate tasks in automation. So--assuming I understand Chris correctly--this issue wouldn't matter for automation because the profile-use build would actually start in a clean directory. It still does matter for local development and testing of anything PGO, because we don't have a good story for implementing the 3-stage system there.

We add to GARBAGE_DIRS in the toplevel Makefile.in for the reasons
described in the comment. We add to GARBAGE_DIRS for Rust programs
because Rust programs currently do not share compilation artifacts with
Rust libraries (as our libraries are built with panic=abort and our
programs are not, sharing compilation artifacts between the two is a
non-starter).

(In reply to Nathan Froyd [:froydnj] from comment #5)

(In reply to Michael Woerister from comment #4)

We're tentatively on track move remaining PGO builds to 3-tier PGO soon. That would probably render this issue obsolete.

Can you tell me more about this? It's unclear to me how this interacts with planned PGO support in the Rust compiler.

The idea behind 3-tier/stage PGO (bug 1507330) is that we want to perform
the instrumented build, the actual profiling, and the profile-use build as
three separate tasks in automation. So--assuming I understand Chris
correctly--this issue wouldn't matter for automation because the profile-use
build would actually start in a clean directory. It still does matter for
local development and testing of anything PGO, because we don't have a good
story for implementing the 3-stage system there.

Yes, that's what I mean. We'll probably want to remove the current targets for doing a PGO build once the remaining builds are converted and implement a small script to run the stages locally for people testing things out.

Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bb96b9e1a32
add Rust compilation directories to GARBAGE_DIRS; r=nalexander

Thanks for the clarification! That sounds like it should not affect the plans for Rust PGO support.

Depends on: 1543325
No longer depends on: 1543325
Regressions: 1543325
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.