Repository navigation
Conversation
`plz query changes` now checks out the merge base of `--since` and HEAD to build the "before" graph, rather than the tip of `--since`. This is consistent with how the changed files are calculated (`--since...HEAD`), so targets that changed on the base branch after the branch point are no longer reported as changed. If the merge base can't be determined (e.g. an unsupported SCM), it warns and falls back to the tip of `--since`. Adds `MergeBase` to the `scm.SCM` interface. Bumps the version to 17.36.0.
`plz query changes` has to parse the whole graph at both the base revision and HEAD, which means checking out the base and parsing it every time. In CI the base is usually the same revision for many runs, so this adds a way to compute it once and reuse it. * `plz query hashes` writes a snapshot of every target's rule hash and tool path hash, plus a header recording the revision, whether the working tree was dirty, the config hash, the hashes of any `pass_env` variables and a fingerprint of the things that make snapshots comparable (format and plz versions, host and target arch, profiles and `-o` overrides). * `plz query changes --before_hashes` diffs against a snapshot instead of checking out and parsing the merge base. The snapshot is rejected if its format or fingerprint differs, it came from a dirty working tree, it wasn't generated at the merge base, or the values of `pass_env` variables differ. We then fall back to parsing the merge base, or fail with `--strict_hashes`. `--force_before_hashes` skips the revision check. * `plz query changes --write_after_hashes` writes a snapshot of HEAD as a by-product, so it can be used as the base for a later run. Snapshots use `encoding/gob`, with maps flattened to sorted slices so identical graphs produce identical files, and are gzipped when the filename ends in `.gz`. `--json` writes a human-readable form for debugging, which can't be read back. `DiffGraphs` is now implemented in terms of `DiffSnapshot`; the tests check the two give identical results.
diffSnapshot computed RuleHash and the tool hash for every target in the after graph sequentially. Do it in chunks across all CPUs instead, sharing the chunking helper with NewSnapshot, and short-circuit to "everything changed" when the config hash differs rather than hashing each target to find that out. On a synthetic graph of ~500k targets this takes diffing from ~850ms to ~340ms. Based on the approach in peter/faster-diffgraphs.
Benchmarks NewSnapshot and diffSnapshot over synthetic graphs of 5k, 50k and 500k targets, with and without ~1% of targets changed. The synthetic graph generator and benchmark layout are taken from peter/faster-diffgraphs. Adds a `changes_benchmark` target using the existing `benchmark` build def, and excludes `*_benchmark_test.go` from `query_test`.
peterebden
reviewed
Nov 6, 2026
| // snapshotFingerprintComponents returns the things that must match for a graph snapshot to be | ||
| // comparable with the current one. The config hash isn't included because a config change is a | ||
| // legitimate change (it marks everything as changed). | ||
| func snapshotFingerprintComponents() map[string]string { |
Collaborator
There was a problem hiding this comment.
I'd ideally like this kind of logic (I think pretty much from here down) to live in a sub-package (this is probably in query somewhere since it's specific to query changes)
| // Note that this is not symmetric; targets that have been removed from 'before' do not appear | ||
| // (because this is designed to be fed into 'plz test' and we can't test targets that no longer exist). | ||
| func DiffGraphs(before, after *core.BuildState, files []string, level int, includeSubrepos bool) core.BuildLabels { | ||
| return DiffSnapshot(NewSnapshot(before), after, files, level, includeSubrepos) |
Collaborator
There was a problem hiding this comment.
FYI in #3636 I moved this out in order to reduce peak memory usage; this will increase it because you'll need to hold the full version of both graphs and the additional snapshot all at once. Ideally the flow is something like
- parse before graph
- derive before snapshot
- drop 'before' state (setting it to nil is enough)
- kick off a GC run
- parse after graph
- run diff
I think that's fairly important, we have hit memory limits here before
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 is based from some of the ideas in https://github.com/Tinder/bazel-diff - the aim here is to allow us to speed up plz query. There's a couple of things going on:
plz query changesused to check out the tip of--since tobuild the "before" graph. It now uses scm.MergeBase(since, HEAD), which matches how the changed files were already calculated. Targets that changed on the base branch after the branch point are no longer reported as changed.plz query hashesparses the whole graph and writes a snapshot. Its flags are --output_file, --json and --print_fingerprint. The last one prints the fingerprint without parsing.plz querychanges gains four flags:--before_hashesreads a snapshot of the merge base instead of checking out and parsing it.--write_after_hashesalso writes a snapshot of HEAD as a by-product, so CI can reuse it later.--force_before_hashesuses a snapshot even if its revision isn't the merge base.--strict_hashesfails instead of falling back when the snapshot is unusable.Parallelise diffing a graph against a snapshot - this is based on this - master...peterebden:please:peter/faster-diffgraphs
Add benchmarks for creating and diffing graph snapshots - again from @peterebden's work