Skip to content

Commit

Permalink
[6.4.0] Also apply NestedSet optimizations to Depset (#19492)
Browse files Browse the repository at this point in the history
If the construction of a `NestedSet` involves only a single non-empty
transitive set, this set may be returned directly as an optimization.
This commit checks for this case when constructing a `Depset` and reuses
the corresponding `Depset` if possible.

Since `Depset`s that are direct elements of providers are usually
unwrapped into their `NestedSet`, this optimization only applies to
instances stored in `struct`s as well as reduces allocations.

Realizes an optimization proposed by @brentleyjones in
https://bazelbuild.slack.com/archives/CA31HN1T3/p1693487067454409?thread_ts=1693484609.534579&cid=CA31HN1T3.

Closes #19379.

Commit
216fce5

PiperOrigin-RevId: 563679197
Change-Id: I542bbf56784832768ca3bbbd550cc78bfb981ffe

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
iancha1992 and fmeum authored Sep 14, 2023
1 parent be2d487 commit 860cb14
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,16 @@ static Depset fromDirectAndTransitive(
if (builder.isEmpty()) {
return builder.getOrder().emptyDepset();
}
return new Depset(type, builder.build());
NestedSet<Object> set = builder.build();
// If the nested set was optimized to one of the transitive elements, reuse the corresponding
// depset.
for (Depset x : transitive) {
if (x.getSet() == set) {
return x;
}
}

return new Depset(type, set);
}

/** An exception thrown when validation fails on the type of elements of a nested set. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,4 +417,17 @@ public void testEmptyDepsetInternedPerOrder() throws Exception {
assertThat(ev.lookup("stable1")).isNotSameInstanceAs(ev.lookup("preorder1"));
assertThat(ev.lookup("stable2")).isNotSameInstanceAs(ev.lookup("preorder2"));
}

@Test
public void testSingleNonEmptyTransitiveAndNoDirectsUnwrapped() throws Exception {
ev.exec(
"inner = depset([1, 2, 3])", "outer = depset(transitive = [depset(), inner, depset()])");
assertThat(ev.lookup("outer")).isSameInstanceAs(ev.lookup("inner"));
}

@Test
public void testSingleNonEmptyTransitiveAndMatchingDirectUnwrapped() throws Exception {
ev.exec("inner = depset([1])", "outer = depset([1], transitive = [depset(), inner, depset()])");
assertThat(ev.lookup("outer")).isSameInstanceAs(ev.lookup("inner"));
}
}

0 comments on commit 860cb14

Please sign in to comment.