Improve computation of data-structures used for include scanning to be more
resource effective. Specifically, only flatten the transitiveHeaderInfo
structure once and use more effective hash maps.
RELNOTES: None.
PiperOrigin-RevId: 248513186
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index 4e41b74..ab44ccc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
@@ -44,6 +44,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
@@ -250,17 +251,22 @@
return headerInfo.textualHeaders;
}
+ public ImmutableList<HeaderInfo> getTransitiveHeaderInfos() {
+ return transitiveHeaderInfos.toList();
+ }
+
public IncludeScanningHeaderData.Builder createIncludeScanningHeaderData(
- boolean usePic, boolean createModularHeaders) {
+ boolean usePic, boolean createModularHeaders, List<HeaderInfo> transitiveHeaderInfoList) {
// We'd prefer for these types to use ImmutableSet/ImmutableMap. However, constructing these is
// substantially more costly in a way that shows up in profiles.
Map<PathFragment, Artifact> pathToLegalOutputArtifact = new HashMap<>();
- ImmutableList<HeaderInfo> infos = transitiveHeaderInfos.toList();
- Set<Artifact> modularHeaders = CompactHashSet.createWithExpectedSize(infos.size());
- for (HeaderInfo transitiveHeaderInfo : infos) {
+ Set<Artifact> modularHeaders =
+ CompactHashSet.createWithExpectedSize(transitiveHeaderInfoList.size());
+ // Not using range-based for loops here and below as the additional overhead of the
+ // ImmutableList iterators has shown up in profiles.
+ for (int c = 0; c < transitiveHeaderInfoList.size(); c++) {
+ HeaderInfo transitiveHeaderInfo = transitiveHeaderInfoList.get(c);
boolean isModule = createModularHeaders && transitiveHeaderInfo.getModule(usePic) != null;
- // Not using range-based for loops here as often there is exactly one element in this list
- // and the amount of garbage created by SingletonImmutableList.iterator() is significant.
for (int i = 0; i < transitiveHeaderInfo.modularHeaders.size(); i++) {
Artifact a = transitiveHeaderInfo.modularHeaders.get(i);
if (!a.isSourceArtifact()) {
@@ -290,8 +296,8 @@
public final Collection<Artifact> modules;
HeadersAndModules(int expectedHeaderCount) {
- headers = new HashSet<>(expectedHeaderCount);
- modules = new LinkedHashSet<>();
+ headers = CompactHashSet.createWithExpectedSize(expectedHeaderCount);
+ modules = CompactHashSet.create();
}
}
@@ -300,9 +306,10 @@
* the modules that they are in.
*/
public HeadersAndModules computeDeclaredHeadersAndUsedModules(
- boolean usePic, Set<Artifact> includes) {
+ boolean usePic, Set<Artifact> includes, List<HeaderInfo> transitiveHeaderInfoList) {
HeadersAndModules result = new HeadersAndModules(includes.size());
- for (HeaderInfo transitiveHeaderInfo : transitiveHeaderInfos) {
+ for (int c = 0; c < transitiveHeaderInfoList.size(); c++) {
+ HeaderInfo transitiveHeaderInfo = transitiveHeaderInfoList.get(c);
Artifact module = transitiveHeaderInfo.getModule(usePic);
// Not using range-based for loops here as often there is exactly one element in this list
// and the amount of garbage created by SingletonImmutableList.iterator() is significant.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index c3dad12..fb520b5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -441,12 +441,15 @@
* sand-boxed environment) and can diagnose the missing headers.
*/
private Iterable<Artifact> filterDiscoveredHeaders(
- ActionExecutionContext actionExecutionContext, Iterable<Artifact> headers) {
+ ActionExecutionContext actionExecutionContext,
+ Iterable<Artifact> headers,
+ List<CcCompilationContext.HeaderInfo> headerInfo) {
Set<Artifact> undeclaredHeaders = Sets.newHashSet(headers);
// Remove all declared headers and find out which modules were used while at it.
CcCompilationContext.HeadersAndModules headersAndModules =
- ccCompilationContext.computeDeclaredHeadersAndUsedModules(usePic, undeclaredHeaders);
+ ccCompilationContext.computeDeclaredHeadersAndUsedModules(
+ usePic, undeclaredHeaders, headerInfo);
usedModules = ImmutableList.copyOf(headersAndModules.modules);
undeclaredHeaders.removeAll(headersAndModules.headers);
@@ -511,11 +514,13 @@
}
commandLineKey = computeCommandLineKey(options);
List<PathFragment> systemIncludeDirs = getSystemIncludeDirs(options);
+ List<CcCompilationContext.HeaderInfo> headerInfo =
+ ccCompilationContext.getTransitiveHeaderInfos();
additionalInputs =
findUsedHeaders(
actionExecutionContext,
ccCompilationContext
- .createIncludeScanningHeaderData(usePic, useHeaderModules)
+ .createIncludeScanningHeaderData(usePic, useHeaderModules, headerInfo)
.setSystemIncludeDirs(systemIncludeDirs)
.setCmdlineIncludes(getCmdlineIncludes(options))
.build());
@@ -529,7 +534,8 @@
if (!shouldScanDotdFiles()) {
// If we aren't looking at .d files later, remove undeclared inputs now.
- additionalInputs = filterDiscoveredHeaders(actionExecutionContext, additionalInputs);
+ additionalInputs =
+ filterDiscoveredHeaders(actionExecutionContext, additionalInputs, headerInfo);
}
}
@@ -1451,7 +1457,8 @@
findUsedHeaders(
actionExecutionContext,
ccCompilationContext
- .createIncludeScanningHeaderData(usePic, useHeaderModules)
+ .createIncludeScanningHeaderData(
+ usePic, useHeaderModules, ccCompilationContext.getTransitiveHeaderInfos())
.setSystemIncludeDirs(getSystemIncludeDirs())
.setCmdlineIncludes(getCmdlineIncludes(getCompilerOptions()))
.build());
