Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nexus shattering 3 of 3: Extract nexus-db-model crate from nexus::db::model #1478

Merged
merged 5 commits into from
Jul 26, 2022

Conversation

jgallagher
Copy link
Contributor

This is "nexus shattering" PR 3 of 3 (for now), which splits the database model into its own crate (nexus-db-model). In addition to the files that were previously under nexus/db/model/, I moved db/schema.rs, db/ipv6.rs, and db/saga_types.rs into nexus-db-model. I moved them primarily because other modules under db/model.rs depend on them, but I think all three of those arguably fit with the model logically anyway; I'd appreciate it if someone could confirm/deny that.

TL;DR: This PR (together with the 2 previous ones) speeds up several cases of both incremental and clean builds by 17-25%, with middling improvements (3-7%) to the remaining cases. At least as of this PR (and arguably before), incremental build times are dominated by linking, and using a (much) faster linker can decrease them by another 50+%. More numbers and analysis below the fold.


Operation main (d023a6d9) this branch (454959e0) this branch using mold
clean build, lib+bin 2m30s,2m29s,2m33s 2m19s,2m24s,2m21s 1m59s,1m59s,1m59s
incremental build, lib+bin, datastore/mod.rs modified 36s,36s,37s 29s,31s,31s 12s,12s,12s
incremental build, lib+bin, model/image.rs modified 36s,36s,37s 35s,35s,35s 17s,17s,16s
clean build, tests 4m39s,4m39s,4m31s 3m45s,3m44s,3m37s 3m17s,3m19s,3m17s
incremental build, tests, datastore/mod.rs modified 1m5s,1m4s,1m5s 48s,48s,49s 20s,21s,21s
incremental build, tests, model/image.rs modified 1m5s,1m5s,1m5s 52s,52s,52s 25s,25s,26s

As with previous timings, in this table "lib+bin" is cargo build -p omicron-nexus and "tests" is cargo build -p omicron-nexus --tests (i.e., the build of the tests without running them). "Modified" means the a space was inserted at the top of the file (which can cause more work than just touch, thanks to issues like rust-lang/rust#47389). Modifying model/image.rs on this branch triggers a rebuild of both nexus-db-model and nexus, as opposed to modifying datastore/mod.rs (which only triggers a rebuild of nexus, not nexus-db-model).

New as of my timings on this branch: I did all the above timings using a 32GiB ramdisk as my target directory and repeated them 3x to check for variance. Before switching to a ramdisk, I was seeing a lot of wild variance and even outright failure (#461) which appears to have been due to poor filesystem performance. A very full filesystem or an ext4 filesystem on an SSD that has had a lot of churn since the last time fstrim has been run can have an outsized impact on build times (I've observed as much as 2x the numbers in the table above for successful builds, plus the builds that failed outright).


I was very surprised to see the full build including tests speed up by nearly a full minute. Repeating that build on both main and this branch (using the stock Linux linker) with --timings, here are screenshots of the end of the build process with the min unit time set to 1.1s and the scale set to 5.

main:

main

this branch:

shatter-model

It looks like there are two factors:

  1. The combined compilation time of nexus-db-model and omicron-nexus on this branch is roughly the same as the time to build omicron-nexus on main, but compiling nexus-db-model is able to start sooner and concurrently with other dependencies of omicron-nexus. (It's hard to tell from the screenshots, but I also think omicron-nexus is able to start slightly before nexus-db-model completes; perhaps rustc doesn't have to wait for the full codegen of a dependency before it starts building the dependent?)
  2. The time to build omicron-nexus lib (test) (the final and longest crate built) decreased by over 30 seconds.

The second point made me realize this is perhaps not a fair comparison, although it might still be the most useful one. On this branch running cargo build -p omicron-nexus --tests no longer builds any tests that moved into the shattered crates. If one is working in nexus proper and not touching the shattered crates, that's probably a good thing. But for a more fair comparison, perhaps cargo build -p omicron-nexus -p nexus-db-model -p nexus-types -p nexus-defaults --tests would be better? This is only a couple seconds slower (3m57s), again due to the shattered nexus-* test crates (which take a combined ~36s, ignoring parallelization) being able to be built concurrently with omicron-nexus itself:

all-shattered

Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a huge improvement! I've got one minor comment below and also asking if there's a TODO here you meant to address before landing this. Anyway, this is looking great and seems like it'll be a big quality of life improvement.

nexus/src/db/mod.rs Outdated Show resolved Hide resolved
nexus/src/db/collection_insert.rs Outdated Show resolved Hide resolved
@jgallagher jgallagher merged commit 5e835f1 into main Jul 26, 2022
@jgallagher jgallagher deleted the shatter-db-model branch July 26, 2022 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants