Skip to content

Commit

Permalink
Add some utilities and relax some visibility restrictions to make alt…
Browse files Browse the repository at this point in the history
…ernative include scanning implementations possible.

--
MOS_MIGRATED_REVID=96337469
  • Loading branch information
janakdr authored and damienmg committed Jun 19, 2015
1 parent 45dae50 commit 42a6830
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2014 Google Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.actions;

import com.google.devtools.build.lib.actions.cache.MetadataHandler;

import java.util.Collection;
import java.util.Map;

/**
* Interface that provides an {@link ActionExecutionContext} on demand. Used to limit the surface
* area available to callers that need to execute an action but do not need the full framework
* normally provided.
*/
public interface ActionExecutionContextFactory {
ActionExecutionContext getContext(ActionInputFileCache graphFileCache,
MetadataHandler metadataHandler, Map<Artifact, Collection<Artifact>> expandedInputMiddlemen);
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ public enum ProfilerTask {
CRITICAL_PATH_COMPONENT("critical path component", -1, 0x666699, 0),
IDE_BUILD_INFO("ide_build_info", -1, 0xCC6633, 0),
HANDLE_GC_NOTIFICATION("gc notification", -1, 0x996633, 0),
INCLUSION_LOOKUP("inclusion lookup", -1, 0x000000, 0),
INCLUSION_PARSE("inclusion parse", -1, 0x000000, 0),
PROCESS_SCAN("process scan", -1, 0x000000, 0),
LOOP_OUTPUT_ARTIFACTS("loop output artifacts"),
LOCATE_RELATIVE("locate relative"),
CONSTRUCT_INCLUDE_PATHS("construct include paths"),
PARSE_AND_HINTS_RESULTS("parse and hints results"),
PROCESS_RESULTS_AND_ENQUEUE("process results and enqueue"),
UNKNOWN("Unknown event", -1, 0x339966, 0);

// Size of the ProfilerTask value space.
Expand All @@ -95,6 +103,10 @@ public enum ProfilerTask {
this.slowestInstancesCount = slowestInstanceCount;
}

ProfilerTask(String description) {
this(description, -1, 0x000000, 0);
}

/** Whether the Profiler collects the slowest instances of this task. */
public boolean collectsSlowestInstances() {
return slowestInstancesCount > 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public interface IncludeScanner {
* interpreted.</p>
*/
void process(Artifact mainSource, Collection<Artifact> sources,
Map<Artifact, Artifact> legalOutputPaths, List<String> cmdlineIncludes,
Map<Artifact, Artifact> legalOutputPaths, List<PathFragment> includeDirs,
List<PathFragment> quoteIncludeDirs, List<String> cmdlineIncludes,
Set<Artifact> includes, ActionExecutionContext actionExecutionContext)
throws IOException, ExecException, InterruptedException;

Expand Down Expand Up @@ -145,8 +146,8 @@ public static Collection<Artifact> scanForIncludedInputs(IncludeScannable action

Artifact mainSource = scannable.getMainIncludeScannerSource();
Collection<Artifact> sources = scannable.getIncludeScannerSources();
scanner.process(mainSource, sources, legalOutputPaths, cmdlineIncludes, includes,
actionExecutionContext);
scanner.process(mainSource, sources, legalOutputPaths, quoteIncludeDirs,
includeDirList, cmdlineIncludes, includes, actionExecutionContext);
}
} catch (IOException e) {
throw new EnvironmentalExecException(e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded(
}
}
actionExecutionContext =
skyframeActionExecutor.constructActionExecutionContext(perActionFileCache,
skyframeActionExecutor.getContext(perActionFileCache,
metadataHandler, state.expandedMiddlemen);
if (!state.hasExecutedAction()) {
state.value = skyframeActionExecutor.executeAction(action,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
public abstract class ArtifactValue implements SkyValue {

@ThreadSafe
static SkyKey key(Artifact artifact, boolean isMandatory) {
public static SkyKey key(Artifact artifact, boolean isMandatory) {
return new SkyKey(SkyFunctions.ARTIFACT, artifact.isSourceArtifact()
? new OwnedArtifact(artifact, isMandatory)
: new OwnedArtifact(artifact));
Expand Down Expand Up @@ -74,6 +74,11 @@ public static Artifact artifact(SkyKey key) {
return TO_ARTIFACT.apply((OwnedArtifact) key.argument());
}

public static boolean equalWithOwner(Artifact first, Artifact second) {
return first.equals(second)
&& first.getArtifactOwner().equals(second.getArtifactOwner());
}

/**
* Artifacts are compared using just their paths, but in Skyframe, the configured target that owns
* an artifact must also be part of the comparison. For example, suppose we build //foo:foo in
Expand All @@ -92,7 +97,7 @@ public static Artifact artifact(SkyKey key) {
* since outside of Skyframe it is quite crucial that Artifacts with different owners be able to
* compare equal.
*/
public static class OwnedArtifact {
static class OwnedArtifact {
private final Artifact artifact;
// Always true for derived artifacts.
private final boolean isMandatory;
Expand Down Expand Up @@ -133,9 +138,7 @@ public boolean equals(Object that) {
}
OwnedArtifact thatOwnedArtifact = ((OwnedArtifact) that);
Artifact thatArtifact = thatOwnedArtifact.artifact;
return artifact.equals(thatArtifact)
&& artifact.getArtifactOwner().equals(thatArtifact.getArtifactOwner())
&& isMandatory == thatOwnedArtifact.isMandatory;
return equalWithOwner(artifact, thatArtifact) && isMandatory == thatOwnedArtifact.isMandatory;
}

Artifact getArtifact() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.Artifact;
Expand All @@ -34,7 +35,13 @@ public class FileArtifactValue extends ArtifactValue {
/** Data for Middleman artifacts that did not have data specified. */
static final FileArtifactValue DEFAULT_MIDDLEMAN = new FileArtifactValue(null, 0, 0);
/** Data that marks that a file is not present on the filesystem. */
static final FileArtifactValue MISSING_FILE_MARKER = new FileArtifactValue(null, 1, 0);
@VisibleForTesting
public static final FileArtifactValue MISSING_FILE_MARKER = new FileArtifactValue(null, 1, 0) {
@Override
public boolean exists() {
return false;
}
};

/**
* Represents an omitted file- we are aware of it but it doesn't exist. All access methods
Expand Down Expand Up @@ -70,7 +77,8 @@ private FileArtifactValue(byte[] digest, long mtime, long size) {
this.mtime = mtime;
}

static FileArtifactValue create(Artifact artifact) throws IOException {
@VisibleForTesting
public static FileArtifactValue create(Artifact artifact) throws IOException {
Path path = artifact.getPath();
FileStatus stat = path.stat();
boolean isFile = stat.isFile();
Expand Down Expand Up @@ -109,7 +117,7 @@ static FileArtifactValue createMiddleman(byte[] digest) {
}

@Nullable
byte[] getDigest() {
public byte[] getDigest() {
return digest;
}

Expand All @@ -119,7 +127,7 @@ boolean isFile() {
}

/** Gets the size of the file. Directories have size 0. */
long getSize() {
public long getSize() {
return size;
}

Expand All @@ -133,6 +141,10 @@ long getModifiedTime() {
return mtime;
}

public boolean exists() {
return true;
}

@Override
public int hashCode() {
// Hash digest by content, not reference. Note that digest is the only array in this array.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
@ThreadSafe
public abstract class FileValue implements SkyValue {

boolean exists() {
public boolean exists() {
return realFileStateValue().getType() != Type.NONEXISTENT;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.actions.ActionCompletionEvent;
import com.google.devtools.build.lib.actions.ActionExecutedEvent;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContextFactory;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionGraph;
Expand Down Expand Up @@ -94,7 +95,7 @@
* Action executor: takes care of preparing an action for execution, executing it, validating that
* all output artifacts were created, error reporting, etc.
*/
public final class SkyframeActionExecutor {
public final class SkyframeActionExecutor implements ActionExecutionContextFactory {
private final Reporter reporter;
private final AtomicReference<EventBus> eventBus;
private final ResourceManager resourceManager;
Expand Down Expand Up @@ -444,8 +445,9 @@ public void expand(Artifact middlemanArtifact, Collection<? super Artifact> outp
* pass the returned context to {@link #executeAction}, and any other method that needs to execute
* tasks related to that action.
*/
ActionExecutionContext constructActionExecutionContext(
PerActionFileCache graphFileCache, MetadataHandler metadataHandler,
@Override
public ActionExecutionContext getContext(
ActionInputFileCache graphFileCache, MetadataHandler metadataHandler,
Map<Artifact, Collection<Artifact>> expandedInputMiddlemen) {
FileOutErr fileOutErr = actionLogBufferPathGenerator.generate();
return new ActionExecutionContext(
Expand Down Expand Up @@ -1054,7 +1056,7 @@ private static class DelegatingPairFileCache implements ActionInputFileCache {
private final ActionInputFileCache perActionCache;
private final ActionInputFileCache perBuildFileCache;

private DelegatingPairFileCache(PerActionFileCache mainCache,
private DelegatingPairFileCache(ActionInputFileCache mainCache,
ActionInputFileCache perBuildFileCache) {
this.perActionCache = mainCache;
this.perBuildFileCache = perBuildFileCache;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
import com.google.devtools.build.lib.actions.ActionExecutionContextFactory;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator;
Expand Down Expand Up @@ -671,6 +672,10 @@ public EventBus getEventBus() {
return eventBus.get();
}

public ActionExecutionContextFactory getActionExecutionContextFactory() {
return skyframeActionExecutor;
}

/**
* The map from package names to the package root where each package was found; this is used to
* set up the symlink tree.
Expand Down

0 comments on commit 42a6830

Please sign in to comment.