From 860cb145ccca064946b6c277986c21ea886d1270 Mon Sep 17 00:00:00 2001 From: "Ian (Hee) Cha" Date: Thu, 14 Sep 2023 10:56:21 -0700 Subject: [PATCH] [6.4.0] Also apply `NestedSet` optimizations to `Depset` (#19492) 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 https://github.com/bazelbuild/bazel/commit/216fce58b81de2c597a78c16514006a4be4891f5 PiperOrigin-RevId: 563679197 Change-Id: I542bbf56784832768ca3bbbd550cc78bfb981ffe Co-authored-by: Fabian Meumertzheim --- .../build/lib/collect/nestedset/Depset.java | 11 ++++++++++- .../build/lib/collect/nestedset/DepsetTest.java | 13 +++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java index 332092a04e0b39..6bf3cd504a43df 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java @@ -392,7 +392,16 @@ static Depset fromDirectAndTransitive( if (builder.isEmpty()) { return builder.getOrder().emptyDepset(); } - return new Depset(type, builder.build()); + NestedSet 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. */ diff --git a/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java b/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java index a2139858beb8f6..3905f90951a970 100644 --- a/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java +++ b/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java @@ -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")); + } }