1551368 - Evaluate not removing rust incremental compilation cache during |mach clobber|

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Eric Rahm [:erahm]]

We'd like to see if it's reasonable to leave the rust incremental compilation cache around after a mach clobber invocation. The idea is that will allow for faster rebuilds when a mach clobber is necessary for C++ code. It's possible there are reasons this won't work, so for now we just want to evaluate the effect on build times and if it's possible at all.

The main drawbacks that I know of are:

1. When we update rust versions we might leave a cache dir around. I don't think this is an actual issue w/in the Firefox build environment.
2. If you're locally hacking a rust checkout and messing with the format of the cache files you could get into a bad state. I don't think we need to worry about this.

Comment 1

[Eric Rahm [:erahm]]

Attachment: Bug 1551368 - WIP: Don't remove the incremental cache when clobbering.

Comment 2

[Eric Rahm [:erahm]]

Attachment: Bug 1551368 - Part 1: Factor out subdirectory logic. r=#build

Pull out the logic for filtering subdirectories and deleting them into
reusable functions.

Comment 3

[Eric Rahm [:erahm]]

Attachment: Bug 1551368 - Part 2: Make the Clobberer aware of substs. r=#build

Pass in the substs dictionary to Clobberer so that we can use it to query cargo paths.

Comment 4

[Eric Rahm [:erahm]]

Attachment: Bug 1551368 - Part 3: Don't remove the rust incremental cache when clobbering. r=#build

Skips over the incremental cache when performing a clobber. The incremental compilation cache is located at:
$(objdir)/$(rust_target)/$(rust_build_kind)/incremental

When cross compiling there can be two caches, one for the host and one for the target so we handle both.

Comment 5

[Eric Rahm [:erahm]]

Initial numbers on this for a pretty contrived use case, essentially:

./mach build && ./mach clobber # fill up ccache and IC cache
multitime -q -n 6 -r './mach clobber' ./mach build # Run 6 times and average results

Shows a 494s -> 386s speedup (~20%).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/028c8f1b6b5b
Part 1: Factor out subdirectory logic. r=firefox-build-system-reviewers,chmanchester

Comment 7

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=028c8f1b6b5b1975255307245a69321adbdac9a8&searchStr=linting%2Copt%2Csource-test-mozlint-py-flake8%2C%28f8%29&selectedJob=250235451

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=250235451&repo=autoland&lineNumber=278

Backout link: https://hg.mozilla.org/integration/autoland/rev/a47c1ea455753afd6dfb1e685c213be931f3bb2b

[task 2019-06-05T21:19:24.821Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2019-06-05T21:19:24.821Z]
[task 2019-06-05T21:19:24.821Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2019-06-05T21:22:37.952Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/python/mozbuild/mozbuild/controller/clobber.py:148:9 | too many blank lines (2) (E303)
[task 2019-06-05T21:22:37.952Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/python/mozbuild/mozbuild/controller/clobber.py:149:5 | expected 1 blank line, found 0 (E301)
[taskcluster 2019-06-05 21:22:38.253Z] === Task Finished ===
[taskcluster 2019-06-05 21:22:40.433Z] Unsuccessful task run with exit code: 1 completed in 501.647 seconds

Comment 8

[Eric Rahm [:erahm]]

I guess lando did a bad rebase, I'll fix locally.

Comment 9

[Eric Rahm [:erahm]]

I tried to lando this but it doesn't seem to have actually landed. I'll leave it to the experts to get this landed.

Comment 10

[Dorel Luca [:dluca]]

Please ask for a review before we can land this.

Removing checkin-needed for now.

Comment 11

[Eric Rahm [:erahm]]

(In reply to Dorel Luca [:dluca] from comment #10)

Please ask for a review before we can land this.

Removing checkin-needed for now.

It has been reviewed, it just need to be rebased.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a8df036968e
Part 1: Factor out subdirectory logic. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/f35cdcfa0a28
Part 2: Make the Clobberer aware of substs. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/33fbb6ff15cd
Part 3: Don't remove the rust incremental cache when clobbering. r=chmanchester

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5a8df036968e
https://hg.mozilla.org/mozilla-central/rev/f35cdcfa0a28
https://hg.mozilla.org/mozilla-central/rev/33fbb6ff15cd