## Motivation
Miniprofs of Zed dropping frames showed foreground thread stalls of
711.56 ms, 641.01 ms, and 221.99 ms from the `edit_file_tool`. The new
benchmark isolated `StreamingDiff::push_new` as one of the expensive
phases so this PR aims to speed it up so we can avoid dropping more
frames in the future.
## Benchmark methodology
I had an agent create a Criterion suite with four deterministic Rust
edit fixtures: tiny localized rewrite, small localized rewrite, many
small changes, and helper block insertions.
Benchmarked with:
```sh
cargo bench -p streaming_diff --bench streaming_diff --profile release-fast -- --warm-up-time 1 --measurement-time 2
```
The benchmark binary was also recorded under `xctrace` CPU Counters to
inspect CPU samples before and after the change.
## Results
`StreamingDiff::push_new` improved across all fixtures:
| Benchmark | Before | After | Improvement |
|---|---:|---:|---:|
| `tiny_function_rewrite` | ~10.81 ms | ~6.91 ms | ~36% faster |
| `small_function_rewrite` | ~51.02 ms | ~35.39 ms | ~31% faster |
| `medium_many_small_changes` | ~130.71 ms | ~83.83 ms | ~36% faster |
| `medium_insertions` | ~120.52 ms | ~79.90 ms | ~34% faster |
The `xctrace` baseline showed samples in `Hash::hash`,
`RandomState::hash_one`, `HashMap::insert`, and
`RawTable::reserve_rehash`. After replacing the map with two `Vec<u32>`
row buffers, the hash table frames disappeared from the top samples.
The speedup comes from removing hash work from the DP inner loop and
replacing scattered hash table probes with contiguous row buffer access
(better cache hits 🏎️). The trace did not include direct cache miss
counts, but this layout is likely more cache friendly because it
replaces hash table access with a continuous line of memory (the
`Vec`s).
Release Notes:
- N/A
We've been considering removing workspace-hack for a couple reasons:
- Lukas ran into a situation where its build script seemed to be causing
spurious rebuilds. This seems more likely to be a cargo bug than an
issue with workspace-hack itself (given that it has an empty build
script), but we don't necessarily want to take the time to hunt that
down right now.
- Marshall mentioned hakari interacts poorly with automated crate
updates (in our case provided by rennovate) because you'd need to have
`cargo hakari generate && cargo hakari manage-deps` after their changes
and we prefer to not have actions that make commits.
Currently removing workspace-hack causes our workspace to grow from
~1700 to ~2000 crates being built (depending on platform), which is
mainly a problem when you're building the whole workspace or running
tests across the the normal and remote binaries (which is where
feature-unification nets us the most sharing). It doesn't impact
incremental times noticeably when you're just iterating on `-p zed`, and
we'll hopefully get these savings back in the future when
rust-lang/cargo#14774 (which re-implements the functionality of hakari)
is finished.
Release Notes:
- N/A
This adds a "workspace-hack" crate, see
[mozilla's](https://hg.mozilla.org/mozilla-central/file/3a265fdc9f33e5946f0ca0a04af73acd7e6d1a39/build/workspace-hack/Cargo.toml#l7)
for a concise explanation of why this is useful. For us in practice this
means that if I were to run all the tests (`cargo nextest r
--workspace`) and then `cargo r`, all the deps from the previous cargo
command will be reused. Before this PR it would rebuild many deps due to
resolving different sets of features for them. For me this frequently
caused long rebuilds when things "should" already be cached.
To avoid manually maintaining our workspace-hack crate, we will use
[cargo hakari](https://docs.rs/cargo-hakari) to update the build files
when there's a necessary change. I've added a step to CI that checks
whether the workspace-hack crate is up to date, and instructs you to
re-run `script/update-workspace-hack` when it fails.
Finally, to make sure that people can still depend on crates in our
workspace without pulling in all the workspace deps, we use a `[patch]`
section following [hakari's
instructions](https://docs.rs/cargo-hakari/0.9.36/cargo_hakari/patch_directive/index.html)
One possible followup task would be making guppy use our
`rust-toolchain.toml` instead of having to duplicate that list in its
config, I opened an issue for that upstream: guppy-rs/guppy#481.
TODO:
- [x] Fix the extension test failure
- [x] Ensure the dev dependencies aren't being unified by Hakari into
the main dependencies
- [x] Ensure that the remote-server binary continues to not depend on
LibSSL
Release Notes:
- N/A
---------
Co-authored-by: Mikayla <mikayla@zed.dev>
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
This PR extracts the streaming diff implementation to its own
`streaming_diff` crate.
It was duplicated between `assistant` and `assistant2`, but their
implementations were exactly the same (and I don't see a reason why they
would need to diverge).
Release Notes:
- N/A
