Skip to content

Fix/empty hash and perf - #25

Merged
dvrkn merged 4 commits into
masterfrom
fix/empty-hash-and-perf
Jul 20, 2026
Merged

dvrkn merged 4 commits into
masterfrom
fix/empty-hash-and-perf

Conversation

@dvrkn

@dvrkn dvrkn commented Jul 20, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes a correctness bug where empty-hash values silently vanish during base extraction, eliminates depth-scaled deep cloning in the three core tree operations, and adds a benchmark suite to lock in the performance baseline.

Correctness: empty-hash values were lost

A key whose value is {} at any depth — e.g. ArgoCD's ubiquitous syncPolicy: {automated: {}} — ended up in neither the base nor any diff. In the hash branch of diff_and_common_multiple, an empty hash contributes no keys, so it produced no base entry and no diff entry and simply disappeared, breaking the merge(base, diff) == original round-trip.

The fix treats the empty hash as an atomic value:

  • it becomes the base value when enough files agree on it (quorum), and
  • otherwise each file holding {} keeps it in its diff.

New tests cover the shared case (all files agree → lands in base), the below-quorum case (outlier file keeps {} in its diff), and the quorum-met-with-outlier case.

Performance: allocation no longer scales with tree depth

Three functions deep-cloned accumulated subtrees once per recursion level, making allocation O(depth × size) instead of O(size):

  • diff_and_common_multiple cloned the accumulated base subtree at every level (sub_base_val.clone().into_owned() — the clone only existed because base_includes_key read sub_base afterwards; reordering removes it)
  • merge_yaml cloned every hash level and re-cloned subtrees along override paths → now clones the base once and merges in place
  • sort_yaml cloned each node and re-cloned children per level, and re-parsed the preOrder config at every hash node → now clones once, sorts in place, parses config once

Public APIs and behavior are unchanged.

Benchmarks

benches/core_benches.rs (criterion, wall time) and benches/alloc_benches.rs (counting global allocator: bytes, allocation count, peak, allocated-bytes/input-size ratio), driven by a synthetic GitOps-values-shaped document generator.

Measured on 87k-node documents (8.5 MiB each):

operation time before → after alloc ratio before → after
diff_and_common_multiple (4 docs) 56.3 ms → 35.9 ms (−36%) 1.86x → 1.04x
diff_and_common_multiple (10 docs) 103.5 ms → 82.5 ms (−20%) —
merge_yaml 29.3 ms → 7.0 ms (−76%) 5.0x → 1.13x
sort_yaml 35.3 ms → 11.75 ms (−70%) 6.2x → 2.08x
compute_diff (untouched) 2.36 ms → 2.34 ms 0.28x (already optimal)

The alloc ratios are now depth-independent. sort_yaml's remaining ~2x is the hash-container rebuild inherent to reordering a LinkedHashMap (constant, not depth-scaled).

Testing

  • cargo test — 26/26 pass, including 3 new empty-hash tests
  • Round-trip validated on a real 4-file GitOps corpus: deep-merging base.yaml with each generated diff structurally reproduces every original file, including the previously-lost syncPolicy blocks
  • cargo bench baselines stored for future regression comparison

dvrkn added 4 commits July 20, 2026 21:08
… deep cloning

- diff_and_common_multiple: a value of {} at any depth (e.g. ArgoCD's
  syncPolicy: {automated: {}}) produced neither base keys nor diff entries
  and was silently dropped. Treat the empty hash as an atomic value: it
  becomes the base when it meets the quorum, otherwise each empty-hash
  file keeps {} in its diff.
- diff_and_common_multiple: move the accumulated base subtree into the
  parent instead of deep-cloning it at every recursion level.
- merge_yaml/sort_yaml: clone the document once and mutate in place
  instead of re-cloning subtrees per recursion level.

Allocation is now depth-independent (diff 1.86x->1.04x of input, merge
5.0x->1.13x, sort 6.2x->2.08x on 87k-node docs); merge -76% and sort -70%
wall time, multi-file diff -20..-36%.
- benches/core_benches.rs: cargo bench baseline for compute_diff,
  diff_and_common_multiple, merge_yaml, sort_yaml at two document sizes.
- benches/alloc_benches.rs: counting global allocator reporting bytes,
  allocation count, peak growth, and allocated-bytes-to-input-size ratio
  per operation; a ratio growing with tree depth indicates redundant
  subtree cloning.
- benches/common: synthetic GitOps-values-shaped document generator.
The old os x arch matrix built natively on every runner, so the arch
dimension only duplicated jobs. Use ubuntu-latest (amd64) and
ubuntu-24.04-arm (arm64) instead.
@dvrkn
dvrkn merged commit 2fc3ed7 into master Jul 20, 2026
2 checks passed
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.

1 participant