Skip to content

Store multi-output build edges once instead of cloning them per output #652

Description

@leynos

Summary

The IR indexes a multi-output target by cloning its complete BuildEdge once for each explicit output.

This remains present on main at ac10b783f33661a0af27bd8f5d19065f095f98db:

  1. manifest lowering creates one outputs: Vec<Utf8PathBuf> of length N;
  2. the vector becomes BuildEdge::explicit_outputs;
  3. insert_edge_for_outputs() inserts edge.clone() for every output except the last;
  4. cloning BuildEdge clones its full explicit_outputs vector and every contained path.

One target with N distinct outputs therefore stores approximately N edges each containing N output paths: O(N²) path storage and clone work before recipe execution.

The original finding noted that the affected conversion was not yet wired into the CLI at its introducing commit. That limiting condition no longer applies: the current generation path loads the manifest and calls BuildGraph::from_manifest_for_shell() during ordinary build/generate operations.

Impact

A compact manifest target with a large output list can amplify into substantial CPU and memory consumption during IR construction. The impact is local/CI availability rather than privilege escalation, but the representation also imposes avoidable costs on legitimate generated manifests and forces downstream consumers to recognize duplicate copies of the same logical edge.

Required change

Represent edge identity separately from output lookup so each logical build edge is owned once.

Suitable models include:

  • an edge arena/vector plus output -> EdgeId index;
  • an interned edge map plus stable edge identifiers;
  • shared immutable ownership such as Arc<BuildEdge> where that does not damage serialization, equality, or public API semantics.

The chosen model should preserve:

  • efficient lookup from any explicit output to its producing edge;
  • one canonical explicit-output vector per logical edge;
  • deterministic iteration and Ninja generation;
  • duplicate-output rejection before mutation of the graph;
  • cycle and missing-dependency analysis across every output alias;
  • clear ownership suitable for future dyndep/implicit-output work.

Prefer a representation that lets Ninja generation iterate canonical edges directly rather than deduplicating values recovered from the output index.

Acceptance criteria

  • A manifest target with N outputs creates one canonical BuildEdge, not N independently owned clones.
  • Total stored path entries attributable to that target grow O(N), with a structural test that does not depend solely on timing.
  • Every output still resolves to the same producer in O(1)-average or equivalently bounded lookup time.
  • Ninja generation emits exactly one build statement containing all explicit outputs.
  • Duplicate outputs within one target and across targets retain deterministic diagnostics.
  • Cycle detection, missing-dependency reporting, graph/help views, dyndep generation, equality tests, and action deduplication retain their current semantics.
  • A stress or benchmark case with thousands of outputs demonstrates linear allocation/work and would fail under the previous cloned-edge representation.
  • The public IR API and design documentation describe canonical edge identity and output indexing.
  • make check-fmt, make lint, make test, and the bounded Kani/property suites pass.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    performancerefactorBehaviour-preserving restructuring that improves code health.testingTest coverage, test infrastructure, and verification tooling work.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions