Skip to content

Commit

Permalink
[clang][modules] Timestamp PCM files when writing (llvm#112452)
Browse files Browse the repository at this point in the history
Clang uses timestamp files to track the last time an implicitly-built
PCM file was verified to be up-to-date with regard to its inputs. With
`-fbuild-session-{file,timestamp}=` and
`-fmodules-validate-once-per-build-session` this reduces the number of
times a PCM file is checked per "build session".

The behavior I'm seeing with the current scheme is that when lots of
Clang instances wait for the same PCM to be built, they race to validate
it as soon as the file lock gets released, causing lots of concurrent
IO.

This patch makes it so that the timestamp is written by the same Clang
instance responsible for building the PCM while still holding the lock.
This makes it so that whenever a PCM file gets compiled, it's never
re-validated in the same build session.

I believe this is as sound as the current scheme. One thing to be aware
of is that there might be a time interval between accessing input file N
and writing the timestamp file, where changes to input files 0..<N would
not result in a rebuild. Since this is the case current scheme too, I'm
not too concerned about that.

I've seen this speed up `clang-scan-deps` by ~27%.
  • Loading branch information
jansvoboda11 authored Oct 22, 2024
1 parent d985197 commit 0ffa29f
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 17 deletions.
5 changes: 3 additions & 2 deletions clang/include/clang/Serialization/ModuleFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#define LLVM_CLANG_SERIALIZATION_MODULEFILE_H

#include "clang/Basic/FileManager.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Serialization/ASTBitCodes.h"
Expand Down Expand Up @@ -144,8 +145,8 @@ class ModuleFile {
/// The base directory of the module.
std::string BaseDirectory;

std::string getTimestampFilename() const {
return FileName + ".timestamp";
static std::string getTimestampFilename(StringRef FileName) {
return (FileName + ".timestamp").str();
}

/// The original source file name that was used to build the
Expand Down
15 changes: 15 additions & 0 deletions clang/lib/Serialization/ASTCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
#include "clang/AST/DeclObjC.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Serialization/ASTDeserializationListener.h"
#include "clang/Serialization/ModuleFile.h"
#include "llvm/Support/DJB.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/raw_ostream.h"

using namespace clang;

Expand Down Expand Up @@ -503,3 +506,15 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
return false;
return isa<TagDecl, FieldDecl>(D);
}

void serialization::updateModuleTimestamp(StringRef ModuleFilename) {
// Overwrite the timestamp file contents so that file's mtime changes.
std::error_code EC;
llvm::raw_fd_ostream OS(ModuleFile::getTimestampFilename(ModuleFilename), EC,
llvm::sys::fs::OF_TextWithCRLF);
if (EC)
return;
OS << "Timestamp file\n";
OS.close();
OS.clear_error(); // Avoid triggering a fatal error.
}
3 changes: 3 additions & 0 deletions clang/lib/Serialization/ASTCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclFriend.h"
#include "clang/Basic/LLVM.h"
#include "clang/Serialization/ASTBitCodes.h"

namespace clang {
Expand Down Expand Up @@ -100,6 +101,8 @@ inline bool isPartOfPerModuleInitializer(const Decl *D) {
return false;
}

void updateModuleTimestamp(StringRef ModuleFilename);

} // namespace serialization

} // namespace clang
Expand Down
15 changes: 1 addition & 14 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4416,19 +4416,6 @@ bool ASTReader::isGlobalIndexUnavailable() const {
!hasGlobalIndex() && TriedLoadingGlobalIndex;
}

static void updateModuleTimestamp(ModuleFile &MF) {
// Overwrite the timestamp file contents so that file's mtime changes.
std::string TimestampFilename = MF.getTimestampFilename();
std::error_code EC;
llvm::raw_fd_ostream OS(TimestampFilename, EC,
llvm::sys::fs::OF_TextWithCRLF);
if (EC)
return;
OS << "Timestamp file\n";
OS.close();
OS.clear_error(); // Avoid triggering a fatal error.
}

/// Given a cursor at the start of an AST file, scan ahead and drop the
/// cursor into the start of the given block ID, returning false on success and
/// true on failure.
Expand Down Expand Up @@ -4707,7 +4694,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type,
ImportedModule &M = Loaded[I];
if (M.Mod->Kind == MK_ImplicitModule &&
M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp)
updateModuleTimestamp(*M.Mod);
updateModuleTimestamp(M.Mod->FileName);
}
}

Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4905,6 +4905,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, StringRef OutputFile,
this->BaseDirectory.clear();

WritingAST = false;

if (WritingModule && SemaRef.PP.getHeaderSearchInfo()
.getHeaderSearchOpts()
.ModulesValidateOncePerBuildSession)
updateModuleTimestamp(OutputFile);

if (ShouldCacheASTInMemory) {
// Construct MemoryBuffer and update buffer manager.
ModuleCache.addBuiltPCM(OutputFile,
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Serialization/ModuleManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
NewModule->InputFilesValidationTimestamp = 0;

if (NewModule->Kind == MK_ImplicitModule) {
std::string TimestampFilename = NewModule->getTimestampFilename();
std::string TimestampFilename =
ModuleFile::getTimestampFilename(NewModule->FileName);
llvm::vfs::Status Status;
// A cached stat value would be fine as well.
if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))
Expand Down

0 comments on commit 0ffa29f

Please sign in to comment.