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

RESOLVED FIXED in Firefox 68

Status

defect
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: mwoerister, Assigned: froydnj)

Tracking

(Blocks 1 bug)

Trunk
mozilla68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(2 attachments)

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

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