Skip to content

Commit

Permalink
Header namespace and transitive HeaderMap
Browse files Browse the repository at this point in the history 
This patch implements transitive headermaps. By using 2 headermaps the
headersearchs are most efficient: a headermap for the current target, and a
header map with namespaced includes.

At the ObjC level, headermaps are created with a namespace of the given target.
In `objc_library` it is possible for the user to override the value of the
namespace via the new attribute, `header_namespace`.

This allows users to include headers from ObjC targets in the convention of
<Namespace/Header.h>

Which is idiomatic for headers inclusions in ObjC development and CocoaPods.

Performance data:

Build time before HeaderMap:
Target //Pinterest/iOS/App:PinterestDevelopment up-to-date:
bazel-bin/Pinterest/iOS/App/PinterestDevelopment.ipa
____Elapsed time: 373.588s, Critical Path: 18.86s

Build time after header maps on the entire project:
Target //Pinterest/iOS/App:PinterestDevelopment up-to-date:
bazel-bin/Pinterest/iOS/App/PinterestDevelopment.ipa
____Elapsed time: 188.971s, Critical Path: 17.11s
  • Loading branch information
@jerrymarino
jerrymarino committed Sep 16, 2017
1 parent f4ffa37 commit af58828
Show file tree
Hide file tree
Showing 9 changed files with 308 additions and 40 deletions.
71 changes: 71 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.LanguageDependentFragment;
Expand Down Expand Up @@ -1367,6 +1368,76 @@ private CppCompilationContext initializeCppCompilationContext(CppModel model) {
}
}

// TODO: Clean this up and have an `experimental` configuration
ImmutableList.Builder<Artifact> headerMapsBuilder = ImmutableList.builder();
String targetName = ruleContext.getTarget().getName();
{
HeaderMapInfo.Builder headerMapInfo = new HeaderMapInfo.Builder();
headerMapInfo.addHeaders(publicHeaders.getHeaders());
headerMapInfo.addHeaders(privateHeaders);
headerMapInfo.addHeaders(publicTextualHeaders);
Root root = ruleContext.getBinOrGenfilesDirectory();
PathFragment path = PathFragment.create(targetName + "_internal.hmap");
Artifact out = ruleContext.getPackageRelativeArtifact(path,
ruleContext
.getConfiguration()
.getGenfilesDirectory(ruleContext.getRule().getRepository()));
ruleContext.registerAction(
new HeaderMapAction(ruleContext.getActionOwner(),
headerMapInfo.build().getSources(),
out));
contextBuilder.addQuoteIncludeDir(out.getExecPath());
headerMapsBuilder.add(out);
}

{
String namespace;
if (ruleContext.attributes().has("header_namespace")) {
namespace = ruleContext.attributes().get("header_namespace", Type.STRING);
} else {
namespace = ruleContext.getRule().getName();
}

HeaderMapInfo.Builder headerMapInfo = new HeaderMapInfo.Builder();
headerMapInfo.setNamespace(namespace);
headerMapInfo.addNamespacedHeaders(publicHeaders.getHeaders());
headerMapInfo.addHeaders(publicHeaders.getHeaders());

// These are all of the headers entered into `srcs`.
headerMapInfo.addNamespacedHeaders(privateHeaders);
headerMapInfo.addHeaders(privateHeaders);

headerMapInfo.addNamespacedHeaders(publicTextualHeaders);
headerMapInfo.addHeaders(publicTextualHeaders);

if (ruleContext.attributes().has("deps")){
for (HeaderMapInfoProvider hmapProvider : ruleContext.getPrerequisites("deps", Mode.TARGET, HeaderMapInfoProvider.class)) {
headerMapInfo.mergeHeaderMapInfo(hmapProvider.getInfo());
}
}
Root root = ruleContext.getBinOrGenfilesDirectory();
PathFragment path = PathFragment.create(targetName + ".hmap");
Artifact out = ruleContext.getPackageRelativeArtifact(path,
ruleContext
.getConfiguration()
.getGenfilesDirectory(ruleContext.getRule().getRepository()));
ruleContext.registerAction(
new HeaderMapAction(ruleContext.getActionOwner(),
headerMapInfo.build().getSources(),
out));
contextBuilder.addIncludeDir(out.getExecPath());
headerMapsBuilder.add(out);
}

ImmutableList headerMaps = headerMapsBuilder.build();
if (headerMaps.size() > 0) {
contextBuilder.setHeaderMaps(headerMaps);
// If we have headermaps, then we need to add an include of
// the working directory ( i.e. exec root ) in this form
// and it must be after including the header map files
contextBuilder.addIncludeDir(PathFragment.create("."));
}

if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS)) {
if (cppModuleMap == null) {
cppModuleMap = CppHelper.createDefaultCppModuleMap(ruleContext, /*suffix=*/ "");
Expand Down
60 changes: 31 additions & 29 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/ClangHeaderMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ enum {
public ByteBuffer buff;

private int numBuckets;
private int numUsedBuckets;
private int stringsOffset;
private int stringsSize;
private int maxValueLength;
Expand All @@ -95,7 +96,6 @@ enum {
// https://clang.llvm.org/doxygen/HeaderMap_8cpp_source.html
ClangHeaderMap(Map<String, String> headerPathsByKeys) {
int dataOffset = 1;
System.out.println(headerPathsByKeys);
setMap(headerPathsByKeys);

int endBuckets = HEADER_SIZE + numBuckets * BUCKET_SIZE;
Expand Down Expand Up @@ -189,55 +189,57 @@ private String getString(int offset) {
}
}

private void addBucket(HMapBucket bucket) {
String key = bucket.key;
int bucketIdx = clangKeyHash(key) & (numBuckets - 1);

// Base case, the bucket Idx is free
if (buckets[bucketIdx] == null) {
buckets[bucketIdx] = bucket;
return;
}

// Handle collisions.

// Try to find the next slot.
//
// The lexer does a linear scan of the hash table when keys do
// not match, starting at the bucket.
while(bucketIdx < numBuckets) {
private void addBucket(HMapBucket bucket, HMapBucket[] buckets, int numBuckets) {
// Use a load factor of 0.5
if (((numUsedBuckets + 1) / numBuckets) > 0.5 == false) {
int bucketIdx = clangKeyHash(bucket.key) & (numBuckets - 1);
// Base case, the bucket Idx is free
if (buckets[bucketIdx] == null) {
buckets[bucketIdx] = bucket;
this.numUsedBuckets++;
return;
}
bucketIdx = (bucketIdx + 1) & (numBuckets - 1);

// Handle collisions.
//
// The lexer does a linear scan of the hash table when keys do
// not match, starting at the bucket.
while(bucketIdx < numBuckets) {
bucketIdx = (bucketIdx + 1) & (numBuckets - 1);
if (buckets[bucketIdx] == null) {
buckets[bucketIdx] = bucket;
this.numUsedBuckets++;
return;
}
}
}

// If there are no more slots left, grow by a power of 2
int newNumBuckets = numBuckets * 2;
HMapBucket[] newBuckets = new HMapBucket[newNumBuckets];
for(int i = 0; i < numBuckets; i++) {
HMapBucket cpBucket = buckets[i];

HMapBucket[] oldBuckets = buckets;
this.buckets = newBuckets;
this.numBuckets = newNumBuckets;
this.numUsedBuckets = 0;
for(HMapBucket cpBucket: oldBuckets) {
if (cpBucket != null) {
int cpBucketIdx = clangKeyHash(cpBucket.key) & (newNumBuckets - 1);
newBuckets[cpBucketIdx] = cpBucket;
addBucket(cpBucket, newBuckets, newNumBuckets);
}
}

buckets = newBuckets;
numBuckets = newNumBuckets;

// Start again
addBucket(bucket);
addBucket(bucket, newBuckets, newNumBuckets);
}

private void setMap(Map<String, String> headerPathsByKeys){
// Compute header metadata
maxValueLength = 1;
maxStringsSize = 0;
numUsedBuckets = 0;

// Per the format, buckets need to be powers of 2 in size
numBuckets = getNextPowerOf2(headerPathsByKeys.size());
numBuckets = getNextPowerOf2(headerPathsByKeys.size() + 1);
buckets = new HMapBucket[numBuckets];

for(Map.Entry<String, String> entry: headerPathsByKeys.entrySet()){
Expand All @@ -258,7 +260,7 @@ private void setMap(Map<String, String> headerPathsByKeys){
}

HMapBucket bucket = new HMapBucket(key, prefix, suffix);
addBucket(bucket);
addBucket(bucket, buckets, numBuckets);
int prefixLen = prefix.getBytes().length + 1;
int suffixLen = suffix.getBytes().length + 1;
int keyLen = key.getBytes().length + 1;
Expand Down
33 changes: 25 additions & 8 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.actions.MiddlemanFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
Expand All @@ -48,11 +49,11 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
public static final CppCompilationContext EMPTY = new Builder(null).build();

private final CommandLineContext commandLineContext;

private final NestedSet<PathFragment> declaredIncludeDirs;
private final NestedSet<PathFragment> declaredIncludeWarnDirs;
private final NestedSet<Artifact> declaredIncludeSrcs;

/**
* Module maps from direct dependencies.
*/
Expand All @@ -62,12 +63,13 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
private final NestedSet<Artifact> nonCodeInputs;

private final NestedSet<Pair<Artifact, Artifact>> pregreppedHdrs;

private final ModuleInfo moduleInfo;
private final ModuleInfo picModuleInfo;

private final CppModuleMap cppModuleMap;
private final CppModuleMap verificationModuleMap;
private final ImmutableList<Artifact> headerMaps;

private final boolean propagateModuleMapAsActionInput;

Expand All @@ -87,7 +89,8 @@ private CppCompilationContext(
NestedSet<Artifact> directModuleMaps,
CppModuleMap cppModuleMap,
@Nullable CppModuleMap verificationModuleMap,
boolean propagateModuleMapAsActionInput) {
boolean propagateModuleMapAsActionInput,
ImmutableList<Artifact> headerMaps) {
Preconditions.checkNotNull(commandLineContext);
this.commandLineContext = commandLineContext;
this.declaredIncludeDirs = declaredIncludeDirs;
Expand All @@ -98,6 +101,7 @@ private CppCompilationContext(
this.moduleInfo = moduleInfo;
this.picModuleInfo = picModuleInfo;
this.cppModuleMap = cppModuleMap;
this.headerMaps = headerMaps;
this.nonCodeInputs = nonCodeInputs;
this.verificationModuleMap = verificationModuleMap;
this.compilationPrerequisites = compilationPrerequisites;
Expand Down Expand Up @@ -231,6 +235,9 @@ public NestedSet<Artifact> getAdditionalInputs() {
if (cppModuleMap != null && propagateModuleMapAsActionInput) {
builder.add(cppModuleMap.getArtifact());
}
if (headerMaps != null) {
builder.addAll(headerMaps);
}
return builder.build();
}

Expand Down Expand Up @@ -279,7 +286,8 @@ public static CppCompilationContext disallowUndeclaredHeaders(CppCompilationCont
context.directModuleMaps,
context.cppModuleMap,
context.verificationModuleMap,
context.propagateModuleMapAsActionInput);
context.propagateModuleMapAsActionInput,
context.headerMaps);
}

/**
Expand Down Expand Up @@ -331,7 +339,8 @@ public static CppCompilationContext mergeForLipo(CppCompilationContext ownerCont
mergeSets(ownerContext.directModuleMaps, libContext.directModuleMaps),
libContext.cppModuleMap,
libContext.verificationModuleMap,
libContext.propagateModuleMapAsActionInput);
libContext.propagateModuleMapAsActionInput,
libContext.headerMaps);
}

/**
Expand Down Expand Up @@ -400,6 +409,8 @@ public static class Builder {
private final Set<String> defines = new LinkedHashSet<>();
private CppModuleMap cppModuleMap;
private CppModuleMap verificationModuleMap;
private ImmutableList<Artifact> headerMaps;

private boolean propagateModuleMapAsActionInput = true;

/** The rule that owns the context */
Expand Down Expand Up @@ -634,6 +645,11 @@ public Builder setCppModuleMap(CppModuleMap cppModuleMap) {
return this;
}

public Builder setHeaderMaps(ImmutableList<Artifact> headerMaps) {
this.headerMaps = headerMaps;
return this;
}

/** Sets the C++ module map used to verify that headers are modules compatible. */
public Builder setVerificationModuleMap(CppModuleMap verificationModuleMap) {
this.verificationModuleMap = verificationModuleMap;
Expand Down Expand Up @@ -704,7 +720,8 @@ public CppCompilationContext build(ActionOwner owner, MiddlemanFactory middleman
directModuleMaps.build(),
cppModuleMap,
verificationModuleMap,
propagateModuleMapAsActionInput);
propagateModuleMapAsActionInput,
headerMaps);
}

/**
Expand Down Expand Up @@ -760,7 +777,7 @@ private Artifact getMiddlemanArtifact(MiddlemanFactory middlemanFactory) {
ruleContext.getRule().getRepository()));
}
}

/**
* Gathers data about the direct and transitive .pcm files belonging to this context. Can be to
* either gather data on PIC or on no-PIC .pcm files.
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderMapAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,12 @@ public String getMnemonic() {
@Override
protected String computeKey() {
Fingerprint f = new Fingerprint();
//TODO: ComputeKeys based on headerMap
f.addString(GUID);
for(Map.Entry<String, String> entry: headerMap.entrySet()){
String key = entry.getKey();
String path = entry.getValue();
f.addString(key + "->" + path);
}
return f.hexDigestAndReset();
}
}
Loading

0 comments on commit af58828

Please sign in to comment.