[meta] a few changes to prevent duplicate dep builds#4535
Merged
sunshowers merged 2 commits intomainfrom Nov 21, 2023
Merged
Conversation
Created using spr 1.3.5
Created using spr 1.3.5
smklein
approved these changes
Nov 21, 2023
Collaborator
smklein
left a comment
There was a problem hiding this comment.
Thanks for doing this investigation and adjustment -- this is a significant boost, happy to see us re-use portions of the build where we can / should!
Contributor
|
Unfortunately, this does not appear to move the needle on illumos.
Incremental compile time measured by adding a comment in the file More detailsSystem
Before full-compileBefore incremental recompileAfter full-compileAfter incremental recompile |
Contributor
Author
|
Discussed this on #oxide-control-plane in Matrix -- unfortunately the listed case in Ry's comment would not benefit (nor be affected negatively) by this change. Where an improvement would occur is with e.g. |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has a few changes that make builds and test runs significantly faster:
xtaskfrom the list of default-members. This makes it so thatcargo nextest runandcargo nextest run -p <package>use more dependency feature sets in common.opt-levelsettings fromprofile.testtoprofile.dev. Again, this results in more cache hits.profile.dev.panictounwind. This is to unify build units across dev and test builds: tests are always built withpanic = "unwind"so that proper backtraces can be printed out. Release builds stay asabort.crdb-seedscript use thetestprofile. If there are any divergences betweendevandtestin the future, then crdb-seed should share its build cache with the tests it was presumably invoked for.profile.dev.build-override.debugtoline-tables-only. This, along with 3, means that target (normal/dev) and build (host) dependencies are now unified.All of this comes together for a pretty sweet improvement.
See #4392 for more details and how I investigated this issue.
Impact
With a fresh build on Linux with mold, I ran three commands in sequence:
cargo nextest run --no-runcargo nextest run -p nexus-db-queriescargo build -p omicron-nexusThe results were:
cargo nextest runcargo nextest run -p nexus-db-queriescargo nextest run -p nexus-db-queriescrdb-seedbuildcargo build -p omicron-nexusSo the cumulative time spent on these three commands went from 354s to 279s. That's a 1.26x speedup. And this should also make other commands better as well (omicron-nexus is a bit of a weird case because it takes a very long time to compile by itself, and that 69s in the "after" column is entirely building omicron-nexus).