Skip to content

bazel/deps: Fix attribution performance and robustness in reachability aspect - #1119

Closed
phlax with Copilot wants to merge 3 commits into
bazel-deps-matrixfrom
copilot/bazel-deps-matrix-another-one
Closed

bazel/deps: Fix attribution performance and robustness in reachability aspect#1119
phlax with Copilot wants to merge 3 commits into
bazel-deps-matrixfrom
copilot/bazel-deps-matrix-another-one

Conversation

Copilot AI commented Aug 24, 2026

Copy link
Copy Markdown

The attribution_patterns feature added per-node dict(repo → depset(package)) propagation through the aspect, causing quadratic-class allocation and unguarded None dereferences on info.repos / info.attributed_packages.

Issue 2 — Per-node dict-of-depsets eliminated

attributed_packages provider field changes from dict(repo → depset(package)) to a flat depset(struct(package=..., repo=...)). Three helpers deleted:

  • _append_repo_package_depsets — copied key lists into a fresh dict at every node for every dep
  • _add_repo_package — built dict entries at matching nodes
  • _materialize_repo_packages — constructed per-repo depsets at every node

Transitive pairs from deps now flow up untouched via transitive=[info.attributed_packages, ...]. The rule impl (which already flattens once per root per config for edges) inverts the flat pair list into the repo → packages mapping at that point:

# aspect: O(1) per non-matching node
attributed_pairs_transitive.append(info.attributed_packages)  # no copy, no flatten

# rule impl: one flatten per root per config
prod_pairs = {}
for p in info.production_attributed_packages.to_list():
    prod_pairs.setdefault(p.repo, {})[p.package] = True
for pair in info.attributed_packages.to_list():
    _record_attributed_package(..., production = pair.package in prod_pairs.get(pair.repo, {}))

Issue 3 — None guards on attribution fields

info.repos, info.production_repos, info.attributed_packages, and info.production_attributed_packages were appended unconditionally. They are None when attribution is disabled. Safe today because _ATTRIBUTION_PATTERNS_SETTING is uniform across the split, but a latent crash otherwise. Explicit if info.repos != None: guards added.

Issue 1 — Documented, not eliminable

The .to_list() at matching nodes cannot be removed in a pure bottom-up aspect without shipping a semantically wrong approximation. Attribution requires pairing each matching package with all repos in its transitive closure, but ancestors haven't been visited yet during bottom-up traversal. The cross-product-at-root alternative (propagate matching_packages and repos as independent depsets, multiply at the rule) incorrectly attributes package B to repo R1 whenever any other matching package A also reaches R1, even if B has no path to R1. The flatten is kept but bounded to nodes whose package actually matches attribution_patterns — a detailed comment explains the constraint.

phlax and others added 2 commits August 24, 2026 23:00
Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve performance and robustness of reachability aspect bazel/deps: Fix attribution performance and robustness in reachability aspect Aug 24, 2026
Copilot AI requested a review from phlax August 24, 2026 22:23
@phlax
phlax force-pushed the bazel-deps-matrix branch 3 times, most recently from 67a14cb to 856fee9 Compare August 25, 2026 13:28
@phlax phlax closed this Aug 25, 2026
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.

2 participants