Use the file size from FileValue instead of wastefully stat'ing the file again in order to get the file size in ParserInputSource#create (used multiple times for each BUILD file and Skylark .bzl file).
--
MOS_MIGRATED_REVID=102930870
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
index fe44617..9bc6bf2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
@@ -52,19 +52,24 @@
/** If {@code lookupSuccessful()}, returns the {@link RootedPath} to the file. */
public abstract RootedPath rootedPath();
+ /** If {@code lookupSuccessful()}, returns the file's size, in bytes. */
+ public abstract long fileSize();
+
static FileLookupResult noFile() {
return UnsuccessfulFileResult.INSTANCE;
}
- static FileLookupResult file(RootedPath rootedPath) {
- return new SuccessfulFileResult(rootedPath);
+ static FileLookupResult file(RootedPath rootedPath, long fileSize) {
+ return new SuccessfulFileResult(rootedPath, fileSize);
}
private static class SuccessfulFileResult extends FileLookupResult {
private final RootedPath rootedPath;
+ private final long fileSize;
- private SuccessfulFileResult(RootedPath rootedPath) {
+ private SuccessfulFileResult(RootedPath rootedPath, long fileSize) {
this.rootedPath = rootedPath;
+ this.fileSize = fileSize;
}
@Override
@@ -76,6 +81,11 @@
public RootedPath rootedPath() {
return rootedPath;
}
+
+ @Override
+ public long fileSize() {
+ return fileSize;
+ }
}
private static class UnsuccessfulFileResult extends FileLookupResult {
@@ -92,6 +102,11 @@
public RootedPath rootedPath() {
throw new IllegalStateException("unsuccessful lookup");
}
+
+ @Override
+ public long fileSize() {
+ throw new IllegalStateException("unsuccessful lookup");
+ }
}
}
@@ -121,12 +136,13 @@
}
BuildFileAST ast = null;
Path path = lookupResult.rootedPath().asPath();
+ long fileSize = lookupResult.fileSize();
// Skylark files end with bzl.
boolean parseAsSkylark = astFilePathFragment.getPathString().endsWith(".bzl");
try {
if (parseAsSkylark) {
try (Mutability mutability = Mutability.create("validate")) {
- ast = BuildFileAST.parseSkylarkFile(path, env.getListener(),
+ ast = BuildFileAST.parseSkylarkFile(path, fileSize, env.getListener(),
packageManager, new ValidationEnvironment(
ruleClassProvider.createSkylarkRuleClassEnvironment(
mutability,
@@ -137,7 +153,7 @@
.setupDynamic(Runtime.PKG_NAME, Runtime.NONE)));
}
} else {
- ast = BuildFileAST.parseBuildFile(path, env.getListener(), packageManager, false);
+ ast = BuildFileAST.parseBuildFile(path, fileSize, env.getListener(), packageManager, false);
}
} catch (IOException e) {
throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(
@@ -177,7 +193,7 @@
return null;
}
if (fileValue.isFile()) {
- return FileLookupResult.file(rootedPath);
+ return FileLookupResult.file(rootedPath, fileValue.getSize());
}
}
return FileLookupResult.noFile();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 12eea02..cf0bf4e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -446,7 +446,7 @@
// Skylark dependencies.
reporter.handle(Event.progress("Loading package: " + packageName));
}
- inputSource = ParserInputSource.create(buildFilePath);
+ inputSource = ParserInputSource.create(buildFilePath, buildFileValue.getSize());
} catch (IOException e) {
env.getListener().handle(Event.error(Location.fromFile(buildFilePath), e.getMessage()));
// Note that we did this work, so we should conservatively report this error as transient.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index ce45ca2..14a40da 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -78,7 +78,7 @@
}
try {
- parser.parse(ParserInputSource.create(repoWorkspace));
+ parser.parse(ParserInputSource.create(repoWorkspace, workspaceFileValue.getSize()));
} catch (IOException e) {
throw new WorkspaceFileFunctionException(e, Transience.TRANSIENT);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index 8dac01e..cdd75d3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -218,7 +218,14 @@
public static BuildFileAST parseBuildFile(Path buildFile, EventHandler eventHandler,
CachingPackageLocator locator, boolean parsePython)
throws IOException {
- ParserInputSource inputSource = ParserInputSource.create(buildFile);
+ return parseBuildFile(buildFile, buildFile.getFileSize(), eventHandler, locator, parsePython);
+ }
+
+ public static BuildFileAST parseBuildFile(Path buildFile, long fileSize,
+ EventHandler eventHandler,
+ CachingPackageLocator locator, boolean parsePython)
+ throws IOException {
+ ParserInputSource inputSource = ParserInputSource.create(buildFile, fileSize);
return parseBuildFile(inputSource, eventHandler, locator, parsePython);
}
@@ -261,7 +268,14 @@
public static BuildFileAST parseSkylarkFile(Path file, EventHandler eventHandler,
CachingPackageLocator locator, ValidationEnvironment validationEnvironment)
throws IOException {
- ParserInputSource input = ParserInputSource.create(file);
+ return parseSkylarkFile(file, file.getFileSize(), eventHandler, locator,
+ validationEnvironment);
+ }
+
+ public static BuildFileAST parseSkylarkFile(Path file, long fileSize, EventHandler eventHandler,
+ CachingPackageLocator locator, ValidationEnvironment validationEnvironment)
+ throws IOException {
+ ParserInputSource input = ParserInputSource.create(file, fileSize);
Lexer lexer = new Lexer(input, eventHandler, false);
Parser.ParseResult result =
Parser.parseFileForSkylark(lexer, eventHandler, locator, validationEnvironment);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java b/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java
index 82b2a44..0877baa 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java
@@ -45,12 +45,19 @@
* all we care about here.
*/
public static ParserInputSource create(Path path) throws IOException {
+ return create(path, path.getFileSize());
+ }
+
+ public static ParserInputSource create(Path path, long fileSize) throws IOException {
+ if (fileSize > Integer.MAX_VALUE) {
+ throw new IOException("Cannot parse file with size larger than 2GB");
+ }
char[] content = FileSystemUtils.readContentAsLatin1(path);
- if (path.getFileSize() > content.length) {
+ if (fileSize > content.length) {
// This assertion is to help diagnose problems arising from the
// filesystem; see bugs and #859334 and #920195.
throw new IOException("Unexpected short read from file '" + path
- + "' (expected " + path.getFileSize() + ", got " + content.length + " bytes)");
+ + "' (expected " + fileSize + ", got " + content.length + " bytes)");
}
return create(content, path.asFragment());
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 7068278..02e1f8b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -664,7 +664,11 @@
*/
protected Rule scratchRule(String packageName, String ruleName, String... lines)
throws Exception {
- scratch.file(packageName + "/BUILD", lines);
+ String buildFilePathString = packageName + "/BUILD";
+ scratch.file(buildFilePathString, lines);
+ skyframeExecutor.invalidateFilesUnderPathForTesting(
+ new ModifiedFileSet.Builder().modify(new PathFragment(buildFilePathString)).build(),
+ rootDirectory);
return (Rule) getTarget("//" + packageName + ":" + ruleName);
}
