Skip to content

Commit 51f3a30

Browse files
committed
[CompileJobCacheKey] Remove the createCompileJobCacheKey() function that accepts an array of arguments
Only expose the variant of `createCompileJobCacheKey()` that accepts a `CompilerInvocation` and does canonicalization for the key calculation, to make sure there is no mistake with a caller passing the wrong set of arguments for the other variant. The only caller of the arguments variant was the dependency scanning daemon, using it to keep track of a global statistics variable `NumCASCacheHit` (by doing an otherwise unnecessary cache key lookup). It is not clear that the daemon keeping track of that variable is important, we could design a better scheme for recording the cache hit rate.
1 parent 4ceab20 commit 51f3a30

File tree

3 files changed

+7
-23
lines changed

3 files changed

+7
-23
lines changed

clang/include/clang/Frontend/CompileJobCacheKey.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,6 @@ llvm::Optional<llvm::cas::CASID>
3434
createCompileJobCacheKey(llvm::cas::CASDB &CAS, DiagnosticsEngine &Diags,
3535
const CompilerInvocation &Invocation);
3636

37-
/// Create a cache key for the given cc1 command-line arguments and filesystem
38-
/// as a \c CASID. The first argument must be "-cc1".
39-
llvm::cas::CASID createCompileJobCacheKey(llvm::cas::CASDB &CAS,
40-
llvm::ArrayRef<const char *> CC1Args,
41-
llvm::cas::CASID FileSystemRootID);
42-
4337
/// Print the structure of the cache key given by \p Key to \p OS. Returns an
4438
/// error if the key object does not exist in \p CAS, or is malformed.
4539
llvm::Error printCompileJobCacheKey(llvm::cas::CASDB &CAS, llvm::cas::CASID Key,

clang/lib/Frontend/CompileJobCacheKey.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ using namespace clang;
2121
using namespace llvm;
2222
using namespace llvm::cas;
2323

24-
llvm::cas::CASID
25-
clang::createCompileJobCacheKey(CASDB &CAS, ArrayRef<const char *> CC1Args,
24+
static llvm::cas::CASID
25+
createCompileJobCacheKeyForArgs(CASDB &CAS, ArrayRef<const char *> CC1Args,
2626
llvm::cas::CASID FileSystemRootID) {
2727
Optional<llvm::cas::ObjectRef> RootRef = CAS.getReference(FileSystemRootID);
2828
if (!RootRef)
@@ -61,8 +61,10 @@ clang::createCompileJobCacheKey(CASDB &CAS, DiagnosticsEngine &Diags,
6161
FrontendOptions &FrontendOpts = InvocationForCacheKey.getFrontendOpts();
6262
DiagnosticOptions &DiagOpts = InvocationForCacheKey.getDiagnosticOpts();
6363
// Keep the key independent of the paths of these outputs.
64-
FrontendOpts.OutputFile = "-";
65-
InvocationForCacheKey.getDependencyOutputOpts().OutputFile = "-";
64+
if (!FrontendOpts.OutputFile.empty())
65+
FrontendOpts.OutputFile = "-";
66+
if (!InvocationForCacheKey.getDependencyOutputOpts().OutputFile.empty())
67+
InvocationForCacheKey.getDependencyOutputOpts().OutputFile = "-";
6668
// We always generate the serialized diagnostics so the key is independent of
6769
// the presence of '--serialize-diagnostics'.
6870
DiagOpts.DiagnosticSerializationFile.clear();
@@ -86,7 +88,7 @@ clang::createCompileJobCacheKey(CASDB &CAS, DiagnosticsEngine &Diags,
8688
return None;
8789
}
8890

89-
return createCompileJobCacheKey(CAS, Argv, *RootID);
91+
return createCompileJobCacheKeyForArgs(CAS, Argv, *RootID);
9092
}
9193

9294
static Error printFileSystem(CASDB &CAS, ObjectRef Ref, raw_ostream &OS) {

clang/tools/driver/cc1depscan_main.cpp

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ using llvm::Error;
6565
#define DEBUG_TYPE "cc1depscand"
6666

6767
ALWAYS_ENABLED_STATISTIC(NumRequests, "Number of -cc1 update requests");
68-
ALWAYS_ENABLED_STATISTIC(NumCASCacheHit,
69-
"Number of compilations should hit cache");
7068

7169
#ifdef CLANG_HAVE_RLIMITS
7270
#if defined(__linux__) && defined(__PIE__)
@@ -1024,16 +1022,6 @@ int cc1depscand_main(ArrayRef<const char *> Argv, const char *Argv0,
10241022
printComputedCC1(OS);
10251023
});
10261024
#endif
1027-
// FIXME: we re-compute the cache key here and try to access the action
1028-
// cache and see if it should be a cache hit. Is there a better way to
1029-
// get this stats in the daemon?
1030-
auto &CAS = Tool->getCachingFileSystem().getCAS();
1031-
auto Key = createCompileJobCacheKey(CAS, NewArgs, *RootID);
1032-
auto Result = CAS.getCachedResult(Key);
1033-
if (Result)
1034-
++NumCASCacheHit;
1035-
else
1036-
llvm::consumeError(Result.takeError());
10371025
}
10381026
});
10391027
};

0 commit comments

Comments
 (0)