1499196 - Share deps among more of the rust in the tree

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Chris Manchester (limited bugmail, email directly)]

We're setting CARGO_TARGET_DIR in order to share deps between libxul and gtest libxul, however we're not for significant amounts of the rust in the tree. In particular we're building libjsrust_shared from bug 1490948 twice.

In the case of libjsrust_shared we're setting CARGO_TARGET_DIR in a fashion that prevents sharing built packages, but even if I fix that we end up building the library and some of its most expensive dependencies twice. Glancing through the fingerprint files it looks like the differences between the two built libjrust_shared libraries are dependencies with different features selected.

Comment 1

[Alex Crichton [:acrichto]]

FWIW we used to hit this in rustc all the time where building Cargo and then building the RLS would rebuild everything (RLS depends on Cargo). We fixed this by doing the following:

* A synthetic crate, rustc-workspace-hack, was published to crates.io
* Both RLS and Cargo were updated to depend on `rustc-workspace-hack`
* A crate was added to rustc at `src/tools/rustc-workspace-hack`
* When building RLS/Cargo there's a `[patch]` section overriding crates.io to this local crate
* The local crate activates a bunch of features for shared dependencies (like winapi)

This ensures that when building locally they're always building with the same set of features. It's workable for now but definitely not the best experience!

Comment 2

[Chris Manchester (limited bugmail, email directly)]

With this hack in place for mozilla central I can save about 90 commands and 1.5 minutes, which is nice. 

Unfortunately we need 7 entries in the placeholder package, so this seems rather brittle against the scenario a new crate with differing features emerges or one of these crates is removed from mozilla central.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1499196 - Always use the topobjdir to output rust library dependencies.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1499196 - Update spidermonkey rust search path to look for rust artifacts rooted in the topobjdir.

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1499196 - Introduce a mozilla-central-workspace-hack crate to unify features seen by rust deps.

This is the equivalent of the rustc-workspace-hack used by the rust build to
ensure cargo and RLS see the same set of features for dependencies so that
these dependencies may be reused by invocations of cargo for these two
projects. The trivial crate added specifies the union of the set of
features activated for a particular crate for each time it appears in the
dependency tree so that cargo will understand these dependencies to be
re-usable across cargo implementations. This eliminates re-building jsrust
and some of its dependencies twice, and reduces the number of crates compiled
in the tree by about 90 in testing on linux.

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1499196 - Use outputs as the key for rust commands in Tup once again.

Now that we're outputing a stub file for each build script we can index these
commands by their output to de-duplicate them in the tup backend.

Comment 7

[(not currently active) Ted Mielczarek]

(In reply to Chris Manchester (:chmanchester) from comment #5)
> re-usable across cargo implementations. This eliminates re-building jsrust
> and some of its dependencies twice, and reduces the number of crates compiled
> in the tree by about 90 in testing on linux.

Nice!

Comment 8

[(not currently active) Ted Mielczarek]

I'll leave that last patch for mshal to review.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/470e06d78a7b
Always use the topobjdir to output rust library dependencies. r=ted,firefox-build-system-reviewers
https://hg.mozilla.org/integration/autoland/rev/98fad0b05551
Update spidermonkey rust search path to look for rust artifacts rooted in the topobjdir. r=bbouvier
https://hg.mozilla.org/integration/autoland/rev/caaad097961a
Introduce a mozilla-central-workspace-hack crate to unify features seen by rust deps. r=ted,firefox-build-system-reviewers
https://hg.mozilla.org/integration/autoland/rev/c43c91d2b97b
Use outputs as the key for rust commands in Tup once again. r=mshal

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/470e06d78a7b
https://hg.mozilla.org/mozilla-central/rev/98fad0b05551
https://hg.mozilla.org/mozilla-central/rev/caaad097961a
https://hg.mozilla.org/mozilla-central/rev/c43c91d2b97b

Comment 11

[Eliza Balazs [:ebalazs_]]

Backed out 4 changesets (Bug 1499196) for regression author didn't respond on time, depends on 1501178, requested by igoldan. a=backout

Backout push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&revision=28fcf5ff82bf4a5d9e3a6425c586d5fcfdf695d2

Comment 12

[Chris Manchester (limited bugmail, email directly)]

I'm actively investigating this and wasn't aware of a deadline for the needinfo. There are circumstances not covered by automation where this is a speed up, so I think re-landing would likely be appropriate, although I'll leave it backed out to take this opportunity to confirm these patches are to blame.

Comment 13

[Byron Campen [:bwc]]

https://hg.mozilla.org/projects/larch/rev/470e06d78a7b1da0aea4fd58b520090097de6e19
Bug 1499196 - Always use the topobjdir to output rust library dependencies. r=ted,firefox-build-system-reviewers

https://hg.mozilla.org/projects/larch/rev/98fad0b05551284eae4d2079123b5555d07dbc4d
Bug 1499196 - Update spidermonkey rust search path to look for rust artifacts rooted in the topobjdir. r=bbouvier

https://hg.mozilla.org/projects/larch/rev/caaad097961a2566dff3742bf57b36085ce3bab6
Bug 1499196 - Introduce a mozilla-central-workspace-hack crate to unify features seen by rust deps. r=ted,firefox-build-system-reviewers

https://hg.mozilla.org/projects/larch/rev/c43c91d2b97b1d1b230efdabce8b67d52e5cda27
Bug 1499196 - Use outputs as the key for rust commands in Tup once again. r=mshal

Comment 14

[Chris Manchester (limited bugmail, email directly)]

The slowdown was caused by the shared output directory reducing the potential for the rust build to parallelize. I would have though cargo's own parallelization would fix this, but it appears lto is enabled for these builds (perhaps unintentionally, these builds are meant to reflect a local build without any particular configuration), and that combined with how the make backend traverses directories containing rust compilation means this created a new bottleneck. I found that by prioritizing these directories in the make backend we can avoid this regression and realize additional gains. I posted a patch to do that in bug 1501178 and will land that along with re-landing these patches.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3181c8c9ea42
Always use the topobjdir to output rust library dependencies. r=ted,firefox-build-system-reviewers
https://hg.mozilla.org/integration/mozilla-inbound/rev/afa1cebbba88
Update spidermonkey rust search path to look for rust artifacts rooted in the topobjdir. r=bbouvier
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1c9e5aec40a
Introduce a mozilla-central-workspace-hack crate to unify features seen by rust deps. r=ted,firefox-build-system-reviewers
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5c6b95843e3
Use outputs as the key for rust commands in Tup once again. r=mshal

Comment 16

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/3181c8c9ea42
https://hg.mozilla.org/mozilla-central/rev/afa1cebbba88
https://hg.mozilla.org/mozilla-central/rev/e1c9e5aec40a
https://hg.mozilla.org/mozilla-central/rev/d5c6b95843e3