Skip to content

Commit

Permalink
Some minor logging improvements related to diff awareness:
Browse files Browse the repository at this point in the history
(i) Log [the first 100] external [non external-repo] file paths when we encounter them in skyframe [when externalFileAction is not ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS]. These caveats are to prevent log spam.

(ii) Fix grammar when logging the results of handleDiffsWithMissingDiffInformation in the case when all package paths have known diff information and we're merely checking external/output/external-repo files.

(iii) Log the purpose of each skyframe graph scan we do (see (ii)).

--
PiperOrigin-RevId: 147508404
MOS_MIGRATED_REVID=147508404
  • Loading branch information
haxorz authored and dslomov committed Feb 15, 2017
1 parent 49705e5 commit fee1928
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
import java.io.IOException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Logger;

/** Common utilities for dealing with paths outside the package roots. */
public class ExternalFilesHelper {
private static final Logger LOG = Logger.getLogger(ExternalFilesHelper.class.getName());
private static final int MAX_NUM_EXTERNAL_FILES_TO_LOG = 100;

private final AtomicReference<PathPackageLocator> pkgLocator;
private final ExternalFileAction externalFileAction;
private final BlazeDirectories directories;
private final AtomicInteger numExternalFilesLogged = new AtomicInteger(0);

// These variables are set to true from multiple threads, but only read in the main thread.
// So volatility or an AtomicBoolean is not needed.
Expand Down Expand Up @@ -179,6 +185,10 @@ void maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment env)
== ExternalFileAction.ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS) {
throw new NonexistentImmutableExternalFileException();
}
if (fileType == FileType.EXTERNAL
&& numExternalFilesLogged.incrementAndGet() < MAX_NUM_EXTERNAL_FILES_TO_LOG) {
LOG.info("Encountered an external path " + rootedPath);
}
return;
}
Preconditions.checkState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,8 @@ private void handleDiffsWithMissingDiffInformation(EventHandler eventHandler,
EnumSet<FileType> fileTypesToCheck = checkOutputFiles
? EnumSet.of(FileType.EXTERNAL, FileType.EXTERNAL_REPO, FileType.OUTPUT)
: EnumSet.of(FileType.EXTERNAL, FileType.EXTERNAL_REPO);
LOG.info("About to scan skyframe graph checking for filesystem nodes of types "
+ Iterables.toString(fileTypesToCheck));
Differencer.Diff diff =
fsvc.getDirtyKeys(
memoizingEvaluator.getValues(),
Expand Down Expand Up @@ -505,8 +507,11 @@ private static void logDiffInfo(Iterable<Path> pathEntries,
int numModified = changedWithNewValue.size() + changedWithoutNewValue.size();
StringBuilder result = new StringBuilder("DiffAwareness found ")
.append(numModified)
.append(" modified source files and directory listings for ")
.append(Joiner.on(", ").join(pathEntries));
.append(" modified source files and directory listings");
if (!Iterables.isEmpty(pathEntries)) {
result.append(" for ");
result.append(Joiner.on(", ").join(pathEntries));
}

if (numModified > 0) {
Iterable<SkyKey> allModifiedKeys = Iterables.concat(changedWithoutNewValue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,8 @@ public SkyKey apply(PathFragment pathFragment) {
// information here, so we compute it ourselves.
// TODO(bazel-team): Fancy filesystems could provide it with a hypothetically modified
// DiffAwareness interface.
LOG.info("About to recompute filesystem nodes corresponding to files that are known to have "
+ "changed");
FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, null);
Map<SkyKey, SkyValue> valuesMap = memoizingEvaluator.getValues();
Differencer.DiffWithDelta diff =
Expand Down

0 comments on commit fee1928

Please sign in to comment.