From 216fce58b81de2c597a78c16514006a4be4891f5 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 8 Sep 2023 01:26:49 -0700 Subject: [PATCH] Also apply `NestedSet` optimizations to `Depset` 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. PiperOrigin-RevId: 563679197 Change-Id: I542bbf56784832768ca3bbbd550cc78bfb981ffe --- .../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 d3ef68b6fc2f41..1d1d6e88a0302d 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 @@ -377,7 +377,16 @@ public 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 87b18b02860b70..2129bf31bb0cf7 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 @@ -419,4 +419,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")); + } }