Skip to content

Commit

Permalink
Roll back ed9d639 due to memory regression.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 315934735
  • Loading branch information
gregestren authored and copybara-github committed Jun 11, 2020
1 parent 0503fee commit 867d232
Show file tree
Hide file tree
Showing 16 changed files with 265 additions and 234 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSet.NestedSetDepthException;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.AdvertisedProviderSet;
Expand Down Expand Up @@ -651,11 +652,23 @@ private static void addSimpleProviders(

private static void assertExecutableSymlinkPresent(
Runfiles runfiles, Artifact executable, Location loc) throws EvalException {
// Extracting the map from Runfiles flattens a depset.
// TODO(cparsons): Investigate: Avoiding this flattening may be an efficiency win.
Map<PathFragment, Artifact> symlinks = runfiles.asMapWithoutRootSymlinks();
if (!symlinks.containsValue(executable)) {
throw new EvalException(loc, "main program " + executable + " not included in runfiles");
try {
// Extracting the map from Runfiles flattens a depset.
// TODO(cparsons): Investigate: Avoiding this flattening may be an efficiency win.
Map<PathFragment, Artifact> symlinks = runfiles.asMapWithoutRootSymlinks();
if (!symlinks.containsValue(executable)) {
throw new EvalException(loc, "main program " + executable + " not included in runfiles");
}
} catch (NestedSetDepthException exception) {
throw new EvalException(
loc,
"depset exceeded maximum depth "
+ exception.getDepthLimit()
+ ". This was only discovered when attempting to flatten the runfiles depset "
+ "returned by the rule implementation function. the size of depsets is unknown "
+ "until flattening. "
+ "See https://github.com/bazelbuild/bazel/issues/9180 for details and possible "
+ "solutions.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.collect.nestedset.NestedSet.NestedSetDepthException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.syntax.Dict;
Expand All @@ -28,7 +29,6 @@
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.StarlarkValue;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import javax.annotation.Nullable;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkDocumentationCategory;
Expand Down Expand Up @@ -341,7 +341,18 @@ public void repr(Printer printer) {
+ "on the depset and vice versa.",
useStarlarkThread = true)
public StarlarkList<Object> toListForStarlark(StarlarkThread thread) throws EvalException {
return StarlarkList.copyOf(thread.mutability(), this.toList());
try {
return StarlarkList.copyOf(thread.mutability(), this.toList());
} catch (NestedSetDepthException exception) {
throw new EvalException(
null,
"depset exceeded maximum depth "
+ exception.getDepthLimit()
+ ". This was only discovered when attempting to flatten the depset for to_list(), "
+ "as the size of depsets is unknown until flattening. "
+ "See https://github.com/bazelbuild/bazel/issues/9180 for details and possible "
+ "solutions.");
}
}

/** Create a Depset from the given direct and transitive components. */
Expand Down Expand Up @@ -548,13 +559,17 @@ public static Depset depset(
result = legacyDepsetConstructor(items, order, direct, transitive, semantics);
}

// check depth limit
int depth = result.getSet().getDepth();
int limit = depthLimit.get();
if (depth > limit) {
throw Starlark.errorf("depset depth %d exceeds limit (%d)", depth, limit);
if (semantics.debugDepsetDepth()) {
// Flatten the underlying nested set. If the set exceeds the depth limit, then this will
// throw a NestedSetDepthException.
// This is an extremely inefficient check and should be only done in the
// "--debug_depset_depth" mode.
try {
result.toList(); // may throw exception
} catch (NestedSetDepthException ex) {
throw Starlark.errorf("depset exceeded maximum depth %d", ex.getDepthLimit());
}
}

return result;
}

Expand Down Expand Up @@ -587,17 +602,4 @@ private static Depset legacyDepsetConstructor(
private static boolean isEmptyStarlarkList(Object o) {
return o instanceof Sequence && ((Sequence) o).isEmpty();
}

/**
* Sets the maximum depth for nested sets constructed by the Starlark {@code depset} function (as
* set by {@code --nested_set_depth_limit}).
*
* @return whether the new limit differs from the old
*/
public static boolean setDepthLimit(int newLimit) {
int oldValue = depthLimit.getAndSet(newLimit);
return oldValue != newLimit;
}

private static final AtomicInteger depthLimit = new AtomicInteger(3500);
}
Loading

0 comments on commit 867d232

Please sign in to comment.