chore: speed up check-features
#481
Conversation
tobz
added
the
type/chore
Makefile
Outdated
| @@ -382,7 +382,7 @@ check-licenses: ## Check that the third-party license file is up to date | |||
| check-features: check-rust-build-tools cargo-install-cargo-hack | |||
| check-features: ## Check that ADP builds with all possible combinations of feature flags | |||
| @echo "[*] Checking feature flag compatibility matrix..." | |||
| @cargo hack check --feature-powerset --tests --quiet | |||
| @cargo hack --feature-powerset -p agent-data-plane -p ddsketch-agent -p saluki-app -p saluki-metrics -p stringtheory -p saluki-tls check --tests --quiet | |||
There was a problem hiding this comment.
What would you think about inverting this and using --exclude instead? That would reduce the risk that we add a new package that has features and forget to include it here.
There was a problem hiding this comment.
Alternatively, this seems to list all packages with features:
$ find . -name Cargo.toml | grep -v '^./Cargo.toml' | xargs -I{} bash -c 'if [[ $(cargo read-manifest --manifest-path {} | jq ".features|del(.default)|length > 0") == "true" ]] ; then echo {} ; fi'
./bin/agent-data-plane/Cargo.toml
./lib/ddsketch-agent/Cargo.toml
./lib/saluki-app/Cargo.toml
./lib/stringtheory/Cargo.toml
./lib/saluki-metrics/Cargo.toml
./lib/saluki-tls/Cargo.toml
So you could use that to build the list to test.
There was a problem hiding this comment.
I'm fine with --exclude.
It's funny... I just went to test this out again because I was about to write "I tried this and for some reason it didn't seem to work"... but then I just tried it again, and it seems to work. 🤷🏻
I like the idea of automatically grabbing all packages with features rather than having to update the list, but it gets a little dicey trying to do more complex logic in Make targets, especially cross-platform... so I think I'll just go with --exclude for now because at least then there's no risk of missing things with features.
There was a problem hiding this comment.
OK, turns out that after typing out the 10th exclude... it was too much for me to stomach and I ended up going with your idea of querying for the crates with features. 😂
There was a problem hiding this comment.
At least it is future proof now :)
Regression Detector (DogStatsD)Regression Detector ResultsRun ID: 845ba64e-1fe1-4709-961b-2b0599f232ec Baseline: 7.63.0-rc.2 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | dsd_uds_100mb_3k_contexts_distributions_only | memory utilization | +0.30 | [+0.13, +0.46] | 1 | |
| ➖ | dsd_uds_10mb_3k_contexts | ingress throughput | +0.00 | [-0.00, +0.01] | 1 | |
| ➖ | dsd_uds_512kb_3k_contexts | ingress throughput | +0.00 | [-0.01, +0.01] | 1 | |
| ➖ | dsd_uds_100mb_250k_contexts | ingress throughput | +0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_1mb_50k_contexts | ingress throughput | +0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_1mb_3k_contexts_dualship | ingress throughput | +0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_1mb_50k_contexts_memlimit | ingress throughput | +0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_1mb_3k_contexts | ingress throughput | -0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_40mb_12k_contexts_40_senders | ingress throughput | -0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_100mb_3k_contexts | ingress throughput | -0.00 | [-0.04, +0.04] | 1 | |
| ➖ | quality_gates_idle_rss | memory utilization | -1.08 | [-1.15, -1.00] | 1 | |
| ➖ | dsd_uds_500mb_3k_contexts | ingress throughput | -2.37 | [-2.46, -2.28] | 1 |
Bounds Checks: ❌ Failed
| perf | experiment | bounds_check_name | replicates_passed | links |
|---|---|---|---|---|
| ❌ | quality_gates_idle_rss | memory_usage | 0/10 |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
Regression Detector (Saluki)Regression Detector ResultsRun ID: b853c959-4ba9-487f-a55e-3a7b2906282c Baseline: b8af303 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | dsd_uds_1mb_50k_contexts_memlimit | ingress throughput | +0.90 | [+0.62, +1.17] | 1 | |
| ➖ | dsd_uds_100mb_250k_contexts | ingress throughput | +0.01 | [-0.03, +0.05] | 1 | |
| ➖ | dsd_uds_50mb_10k_contexts_no_inlining | ingress throughput | +0.01 | [-0.06, +0.07] | 1 | |
| ➖ | dsd_uds_1mb_3k_contexts_dualship | ingress throughput | +0.00 | [-0.00, +0.00] | 1 | |
| ➖ | dsd_uds_512kb_3k_contexts | ingress throughput | +0.00 | [-0.01, +0.01] | 1 | |
| ➖ | dsd_uds_1mb_50k_contexts | ingress throughput | -0.00 | [-0.02, +0.02] | 1 | |
| ➖ | dsd_uds_1mb_3k_contexts | ingress throughput | -0.00 | [-0.01, +0.01] | 1 | |
| ➖ | dsd_uds_100mb_3k_contexts | ingress throughput | -0.00 | [-0.04, +0.04] | 1 | |
| ➖ | dsd_uds_40mb_12k_contexts_40_senders | ingress throughput | -0.01 | [-0.03, +0.02] | 1 | |
| ➖ | dsd_uds_50mb_10k_contexts_no_inlining_no_allocs | ingress throughput | -0.01 | [-0.06, +0.05] | 1 | |
| ➖ | dsd_uds_10mb_3k_contexts | ingress throughput | -0.02 | [-0.05, +0.01] | 1 | |
| ➖ | quality_gates_idle_rss | memory utilization | -0.16 | [-0.19, -0.13] | 1 | |
| ➖ | dsd_uds_100mb_3k_contexts_distributions_only | memory utilization | -0.40 | [-0.50, -0.29] | 1 | |
| ➖ | dsd_uds_500mb_3k_contexts | ingress throughput | -0.96 | [-1.08, -0.84] | 1 |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | links |
|---|---|---|---|---|
| ✅ | quality_gates_idle_rss | memory_usage | 10/10 |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
Regression Detector LinksExperiment Result Links
|
tobz
force-pushed
the
tobz/improve-check-features-ci-job
branch
from
c4bde5d to
daa0905
Compare
Summary
The
check-featuresMake target, and subsequently the CI job for it, is slow. Takes roughly 6-7 minutes on a clean build. This isn't great.A lot of this is because we use a lot of dependencies, which is hard to get around... but a significant chunk -- around 30% -- is actually because we're doing
cargo checkon crates with no features... which is wasted time and effort.This PR simply scopes
cargo hack checkdirectly to the crates with features that need to be checked, since the rest of the crates not being checked will be checked elsewhere: during unit tests, when the binaries they generate are used, and so on.When testing locally, a clean run of
check-featurestook 6m20s on my machine before this PR. After the PR, it takes around 4m20s... which is around 32% faster overall.Change Type
How did you test this PR?
N/A
References
N/A