Reduce visibility of nestedset-on-skyframe members.
PiperOrigin-RevId: 314766990
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index ae7bcbd..c03916d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -411,7 +411,7 @@
* necessary. The default case (without --experimental_nestedset_as_skykey_threshold) will ignore
* this path.
*/
- public static boolean evalInputsAsNestedSet(NestedSet<Artifact> inputs) {
+ private static boolean evalInputsAsNestedSet(NestedSet<Artifact> inputs) {
int nestedSetSizeThreshold = ArtifactNestedSetFunction.getSizeThreshold();
if (nestedSetSizeThreshold == 1) {
// Don't even flatten in this case.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
index 2ae5336..be9eb65 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
@@ -49,7 +49,7 @@
* <p>[1] Heuristic: If the size of the NestedSet exceeds a certain threshold, we evaluate it as an
* ArtifactNestedSetKey.
*/
-public class ArtifactNestedSetFunction implements SkyFunction {
+final class ArtifactNestedSetFunction implements SkyFunction {
/**
* A concurrent map from Artifacts' SkyKeys to their ValueOrException, for Artifacts that are part
@@ -83,7 +83,7 @@
* will GCed immediately. // TODO(leba): Re-evaluate the above point about weak-valued map.
*
* <p>This map will be removed when --experimental_nsos_eval_keys_as_one_group is stable, and
- * replaced by {@link artifactSkyKeyToSkyValue}.
+ * replaced by {@link #artifactSkyKeyToSkyValue}.
*/
private ConcurrentMap<
SkyKey,
@@ -93,7 +93,7 @@
/**
* A concurrent map from Artifacts' SkyKeys to their SkyValue, for Artifacts that are part of
* NestedSets which were evaluated as {@link ArtifactNestedSetKey}. This is expected to replace
- * the above {@link artifactSkyKeyToValueOrException}.
+ * the above {@link #artifactSkyKeyToValueOrException}.
*/
private ConcurrentMap<SkyKey, SkyValue> artifactSkyKeyToSkyValue;
@@ -220,7 +220,7 @@
return new ArtifactNestedSetValue();
}
- public static ArtifactNestedSetFunction getInstance() {
+ static ArtifactNestedSetFunction getInstance() {
if (singleton == null) {
return createInstance();
}
@@ -232,13 +232,13 @@
* this method separated from {@code #getInstance} since sometimes we need to overwrite the
* existing instance.
*/
- public static ArtifactNestedSetFunction createInstance() {
+ static ArtifactNestedSetFunction createInstance() {
singleton = new ArtifactNestedSetFunction();
return singleton;
}
/** Reset the various state-keeping maps of ArtifactNestedSetFunction. */
- public void resetArtifactNestedSetFunctionMaps() {
+ void resetArtifactNestedSetFunctionMaps() {
artifactSkyKeyToValueOrException = Maps.newConcurrentMap();
artifactSkyKeyToSkyValue = Maps.newConcurrentMap();
}
@@ -247,7 +247,7 @@
return artifactSkyKeyToSkyValue;
}
- public Map<
+ Map<
SkyKey,
ValueOrException3<IOException, ActionExecutionException, ArtifactNestedSetEvalException>>
getArtifactSkyKeyToValueOrException() {
@@ -263,7 +263,7 @@
* Get the threshold to which we evaluate a NestedSet as a Skykey. If sizeThreshold is unset,
* return the default value of 0.
*/
- public static int getSizeThreshold() {
+ static int getSizeThreshold() {
return sizeThreshold == null ? 0 : sizeThreshold;
}
@@ -276,7 +276,7 @@
*
* @return whether an update was made.
*/
- public static boolean evalKeysAsOneGroupUpdated(boolean newEvalKeysAsOneGroup) {
+ static boolean evalKeysAsOneGroupUpdated(boolean newEvalKeysAsOneGroup) {
// If this is the first time the value is set, it's not considered "updated".
if (evalKeysAsOneGroup == null) {
evalKeysAsOneGroup = newEvalKeysAsOneGroup;
@@ -297,7 +297,7 @@
* @param newValue The new value from --experimental_nested_set_as_skykey_threshold.
* @return whether an update was made.
*/
- public static boolean sizeThresholdUpdated(int newValue) {
+ static boolean sizeThresholdUpdated(int newValue) {
// If this is the first time the value is set, it's not considered "updated".
if (sizeThreshold == null) {
sizeThreshold = newValue;
@@ -335,7 +335,7 @@
this.nestedExceptions = nestedExceptions;
}
- public NestedSet<Pair<SkyKey, Exception>> getNestedExceptions() {
+ NestedSet<Pair<SkyKey, Exception>> getNestedExceptions() {
return nestedExceptions;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java
index b2c86db..b2ed5b7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -23,7 +22,7 @@
import com.google.devtools.build.skyframe.SkyKey;
/** SkyKey for {@code NestedSet<Artifact>}. */
-public class ArtifactNestedSetKey implements SkyKey {
+final class ArtifactNestedSetKey implements SkyKey {
private final Object rawChildren;
@Override
@@ -37,16 +36,11 @@
*
* @param rawChildren the underlying members of the nested set.
*/
- public ArtifactNestedSetKey(Object rawChildren) {
+ ArtifactNestedSetKey(Object rawChildren) {
Preconditions.checkState(rawChildren instanceof Object[] || rawChildren instanceof Artifact);
this.rawChildren = rawChildren;
}
- @VisibleForTesting
- public Object getRawChildrenForTesting() {
- return rawChildren;
- }
-
@Override
public int hashCode() {
return rawChildren.hashCode();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index c4e80d1..cc42bb1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -274,6 +274,23 @@
}
/**
+ * Updates ArtifactNestedSetFunction options if the flags' values changed.
+ *
+ * @return whether an update was made.
+ */
+ private static boolean nestedSetAsSkyKeyOptionsChanged(OptionsProvider options) {
+ BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class);
+ if (buildRequestOptions == null) {
+ return false;
+ }
+
+ return ArtifactNestedSetFunction.sizeThresholdUpdated(
+ buildRequestOptions.nestedSetAsSkyKeyThreshold)
+ || ArtifactNestedSetFunction.evalKeysAsOneGroupUpdated(
+ buildRequestOptions.nsosEvalKeysAsOneGroup);
+ }
+
+ /**
* The value types whose builders have direct access to the package locator, rather than accessing
* it via an explicit Skyframe dependency. They need to be invalidated if the package locator
* changes.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 5f81935..9b59e0d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -2715,23 +2715,6 @@
}
}
- /**
- * Updates ArtifactNestedSetFunction options if the flags' values changed.
- *
- * @return whether an update was made.
- */
- protected static boolean nestedSetAsSkyKeyOptionsChanged(OptionsProvider options) {
- BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class);
- if (buildRequestOptions == null) {
- return false;
- }
-
- return ArtifactNestedSetFunction.sizeThresholdUpdated(
- buildRequestOptions.nestedSetAsSkyKeyThreshold)
- || ArtifactNestedSetFunction.evalKeysAsOneGroupUpdated(
- buildRequestOptions.nsosEvalKeysAsOneGroup);
- }
-
protected void syncPackageLoading(
PackageOptions packageOptions,
PathPackageLocator pathPackageLocator,
diff --git a/src/main/java/com/google/devtools/build/lib/util/GroupedList.java b/src/main/java/com/google/devtools/build/lib/util/GroupedList.java
index 397b2d9..f09a8fd 100644
--- a/src/main/java/com/google/devtools/build/lib/util/GroupedList.java
+++ b/src/main/java/com/google/devtools/build/lib/util/GroupedList.java
@@ -384,32 +384,6 @@
};
}
- /** Creates an already compressed {@code GroupedList} with four groups. */
- public static <E> @Compressed Object createCompressedWithFourGroups(
- E singletonElementOfFirstGroup,
- List<? extends E> elementsOfSecondGroup,
- List<? extends E> elementsOfThirdGroup,
- List<? extends E> elementsOfFourthGroup) {
- if (elementsOfSecondGroup.isEmpty()) {
- return createCompressedWithThreeGroups(
- singletonElementOfFirstGroup, elementsOfThirdGroup, elementsOfFourthGroup);
- }
- if (elementsOfThirdGroup.isEmpty()) {
- return createCompressedWithThreeGroups(
- singletonElementOfFirstGroup, elementsOfSecondGroup, elementsOfFourthGroup);
- }
- if (elementsOfFourthGroup.isEmpty()) {
- return createCompressedWithThreeGroups(
- singletonElementOfFirstGroup, elementsOfSecondGroup, elementsOfThirdGroup);
- }
- return new Object[] {
- singletonElementOfFirstGroup,
- singleElementOrList(elementsOfSecondGroup),
- singleElementOrList(elementsOfThirdGroup),
- singleElementOrList(elementsOfFourthGroup),
- };
- }
-
private static Object singleElementOrList(List<?> list) {
return list.size() == 1 ? list.get(0) : list;
}
diff --git a/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java b/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java
index 0b5d662..5eac369 100644
--- a/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java
@@ -152,7 +152,7 @@
Object compressed = groupedList.compress();
assertThat(GroupedList.numElements(compressed)).isEqualTo(groupedList.numElements());
assertElementsEqual(compressed, allElts, elements.size());
- assertElementsEqualInGroups(GroupedList.<String>create(compressed), elements);
+ assertElementsEqualInGroups(GroupedList.create(compressed), elements);
assertElementsEqualInGroups(groupedList, elements);
assertThat(groupedList.getAllElementsAsIterable())
.containsExactlyElementsIn(Iterables.concat(groupedList))
@@ -164,13 +164,12 @@
GroupedList<String> groupedList = new GroupedList<>();
assertThat(groupedList.isEmpty()).isTrue();
GroupedListHelper<String> helper = new GroupedListHelper<>();
- @SuppressWarnings("unchecked") // varargs
- List<ImmutableList<String>> elements = Lists.newArrayList(
- ImmutableList.of("1"),
- ImmutableList.<String>of(),
- ImmutableList.of("2a", "2b"),
- ImmutableList.of("3")
- );
+ List<ImmutableList<String>> elements =
+ Lists.newArrayList(
+ ImmutableList.of("1"),
+ ImmutableList.of(),
+ ImmutableList.of("2a", "2b"),
+ ImmutableList.of("3"));
List<String> allElts = new ArrayList<>();
for (List<String> group : elements) {
helper.startGroup(); // Start a group even if the group has only one element or is empty.
@@ -187,7 +186,7 @@
assertElementsEqual(compressed, allElts, 3);
// Get rid of empty list -- it was not stored in groupedList.
elements.remove(1);
- assertElementsEqualInGroups(GroupedList.<String>create(compressed), elements);
+ assertElementsEqualInGroups(GroupedList.create(compressed), elements);
assertElementsEqualInGroups(groupedList, elements);
assertThat(groupedList.getAllElementsAsIterable())
.containsExactlyElementsIn(Iterables.concat(groupedList))
@@ -242,35 +241,18 @@
}
@Test
- public void createCompressedWithFourGroups() {
- GroupedList<String> groupedList = new GroupedList<>();
- groupedList.appendGroup(ImmutableList.of("a"));
- groupedList.appendGroup(ImmutableList.of("b", "c"));
- groupedList.appendGroup(ImmutableList.of("d", "e", "f"));
- groupedList.appendGroup(ImmutableList.of("g", "h", "i", "j"));
- assertThat(
- GroupedList.createCompressedWithFourGroups(
- "a",
- ImmutableList.of("b", "c"),
- ImmutableList.of("d", "e", "f"),
- ImmutableList.of("g", "h", "i", "j")))
- .isEqualTo(groupedList.compress());
- }
-
- @Test
public void removeMakesEmpty() {
GroupedList<String> groupedList = new GroupedList<>();
assertThat(groupedList.isEmpty()).isTrue();
GroupedListHelper<String> helper = new GroupedListHelper<>();
- @SuppressWarnings("unchecked") // varargs
- List<List<String>> elements = Lists.newArrayList(
- (List<String>) ImmutableList.of("1"),
- ImmutableList.<String>of(),
- Lists.newArrayList("2a", "2b"),
- ImmutableList.of("3"),
- ImmutableList.of("removedGroup1", "removedGroup2"),
- ImmutableList.of("4")
- );
+ List<List<String>> elements =
+ Lists.newArrayList(
+ ImmutableList.of("1"),
+ ImmutableList.of(),
+ Lists.newArrayList("2a", "2b"),
+ ImmutableList.of("3"),
+ ImmutableList.of("removedGroup1", "removedGroup2"),
+ ImmutableList.of("4"));
List<String> allElts = new ArrayList<>();
for (List<String> group : elements) {
helper.startGroup(); // Start a group even if the group has only one element or is empty.
@@ -291,7 +273,7 @@
elements.remove(ImmutableList.of("3"));
elements.remove(ImmutableList.of());
elements.remove(ImmutableList.of("removedGroup1", "removedGroup2"));
- assertElementsEqualInGroups(GroupedList.<String>create(compressed), elements);
+ assertElementsEqualInGroups(GroupedList.create(compressed), elements);
assertElementsEqualInGroups(groupedList, elements);
}
@@ -318,7 +300,7 @@
allElts.removeAll(removed);
assertElementsEqual(compressed, allElts, 1);
elements.get(0).removeAll(removed);
- assertElementsEqualInGroups(GroupedList.<String>create(compressed), elements);
+ assertElementsEqualInGroups(GroupedList.create(compressed), elements);
assertElementsEqualInGroups(groupedList, elements);
}
