[clang] e80ee18 - Reland [Frontend] avoid copy of PCH data when PrecompiledPreamble stores it in memory
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 21 12:16:47 PDT 2022
Author: Sam McCall
Date: 2022-04-21T21:15:39+02:00
New Revision: e80ee1829c59b34a574cd424ecdcbdff400f1401
URL: https://github.com/llvm/llvm-project/commit/e80ee1829c59b34a574cd424ecdcbdff400f1401
DIFF: https://github.com/llvm/llvm-project/commit/e80ee1829c59b34a574cd424ecdcbdff400f1401.diff
LOG: Reland [Frontend] avoid copy of PCH data when PrecompiledPreamble stores it in memory
This reverts commit eadf35270727ca743c11b07040bbfedd415ab6dc.
The reland fixes a couple of places in clang that were unneccesarily
requesting a null-terminated buffer of the PCH, and hitting assertions.
Added:
Modified:
clang/lib/Frontend/PrecompiledPreamble.cpp
clang/lib/Serialization/ASTReader.cpp
Removed:
################################################################################
diff --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp
index 9f5be05a14308..e8128e3828edd 100644
--- a/clang/lib/Frontend/PrecompiledPreamble.cpp
+++ b/clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -234,9 +234,10 @@ class TempPCHFile {
class PrecompilePreambleAction : public ASTFrontendAction {
public:
- PrecompilePreambleAction(std::string *InMemStorage,
+ PrecompilePreambleAction(std::shared_ptr<PCHBuffer> Buffer, bool WritePCHFile,
PreambleCallbacks &Callbacks)
- : InMemStorage(InMemStorage), Callbacks(Callbacks) {}
+ : Buffer(std::move(Buffer)), WritePCHFile(WritePCHFile),
+ Callbacks(Callbacks) {}
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override;
@@ -244,6 +245,12 @@ class PrecompilePreambleAction : public ASTFrontendAction {
bool hasEmittedPreamblePCH() const { return HasEmittedPreamblePCH; }
void setEmittedPreamblePCH(ASTWriter &Writer) {
+ if (FileOS) {
+ *FileOS << Buffer->Data;
+ // Make sure it hits disk now.
+ FileOS->flush();
+ }
+
this->HasEmittedPreamblePCH = true;
Callbacks.AfterPCHEmitted(Writer);
}
@@ -262,7 +269,9 @@ class PrecompilePreambleAction : public ASTFrontendAction {
friend class PrecompilePreambleConsumer;
bool HasEmittedPreamblePCH = false;
- std::string *InMemStorage;
+ std::shared_ptr<PCHBuffer> Buffer;
+ bool WritePCHFile; // otherwise the PCH is written into the PCHBuffer only.
+ std::unique_ptr<llvm::raw_pwrite_stream> FileOS; // null if in-memory
PreambleCallbacks &Callbacks;
};
@@ -272,12 +281,11 @@ class PrecompilePreambleConsumer : public PCHGenerator {
const Preprocessor &PP,
InMemoryModuleCache &ModuleCache,
StringRef isysroot,
- std::unique_ptr<raw_ostream> Out)
- : PCHGenerator(PP, ModuleCache, "", isysroot,
- std::make_shared<PCHBuffer>(),
+ std::shared_ptr<PCHBuffer> Buffer)
+ : PCHGenerator(PP, ModuleCache, "", isysroot, std::move(Buffer),
ArrayRef<std::shared_ptr<ModuleFileExtension>>(),
/*AllowASTWithErrors=*/true),
- Action(Action), Out(std::move(Out)) {}
+ Action(Action) {}
bool HandleTopLevelDecl(DeclGroupRef DG) override {
Action.Callbacks.HandleTopLevelDecl(DG);
@@ -288,15 +296,6 @@ class PrecompilePreambleConsumer : public PCHGenerator {
PCHGenerator::HandleTranslationUnit(Ctx);
if (!hasEmittedPCH())
return;
-
- // Write the generated bitstream to "Out".
- *Out << getPCH();
- // Make sure it hits disk now.
- Out->flush();
- // Free the buffer.
- llvm::SmallVector<char, 0> Empty;
- getPCH() = std::move(Empty);
-
Action.setEmittedPreamblePCH(getWriter());
}
@@ -306,7 +305,6 @@ class PrecompilePreambleConsumer : public PCHGenerator {
private:
PrecompilePreambleAction &Action;
- std::unique_ptr<raw_ostream> Out;
};
std::unique_ptr<ASTConsumer>
@@ -316,21 +314,18 @@ PrecompilePreambleAction::CreateASTConsumer(CompilerInstance &CI,
if (!GeneratePCHAction::ComputeASTConsumerArguments(CI, Sysroot))
return nullptr;
- std::unique_ptr<llvm::raw_ostream> OS;
- if (InMemStorage) {
- OS = std::make_unique<llvm::raw_string_ostream>(*InMemStorage);
- } else {
- std::string OutputFile;
- OS = GeneratePCHAction::CreateOutputFile(CI, InFile, OutputFile);
+ if (WritePCHFile) {
+ std::string OutputFile; // unused
+ FileOS = GeneratePCHAction::CreateOutputFile(CI, InFile, OutputFile);
+ if (!FileOS)
+ return nullptr;
}
- if (!OS)
- return nullptr;
if (!CI.getFrontendOpts().RelocatablePCH)
Sysroot.clear();
return std::make_unique<PrecompilePreambleConsumer>(
- *this, CI.getPreprocessor(), CI.getModuleCache(), Sysroot, std::move(OS));
+ *this, CI.getPreprocessor(), CI.getModuleCache(), Sysroot, Buffer);
}
template <class T> bool moveOnNoError(llvm::ErrorOr<T> Val, T &Output) {
@@ -356,9 +351,9 @@ class PrecompiledPreamble::PCHStorage {
S->File = std::move(File);
return S;
}
- static std::unique_ptr<PCHStorage> inMemory() {
+ static std::unique_ptr<PCHStorage> inMemory(std::shared_ptr<PCHBuffer> Buf) {
std::unique_ptr<PCHStorage> S(new PCHStorage());
- S->Memory.emplace();
+ S->Memory = std::move(Buf);
return S;
}
@@ -376,11 +371,7 @@ class PrecompiledPreamble::PCHStorage {
}
llvm::StringRef memoryContents() const {
assert(getKind() == Kind::InMemory);
- return *Memory;
- }
- std::string &memoryBufferForWrite() {
- assert(getKind() == Kind::InMemory);
- return *Memory;
+ return StringRef(Memory->Data.data(), Memory->Data.size());
}
private:
@@ -388,7 +379,7 @@ class PrecompiledPreamble::PCHStorage {
PCHStorage(const PCHStorage &) = delete;
PCHStorage &operator=(const PCHStorage &) = delete;
- llvm::Optional<std::string> Memory;
+ std::shared_ptr<PCHBuffer> Memory;
std::unique_ptr<TempPCHFile> File;
};
@@ -411,9 +402,10 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
PreprocessorOptions &PreprocessorOpts =
PreambleInvocation->getPreprocessorOpts();
+ std::shared_ptr<PCHBuffer> Buffer = std::make_shared<PCHBuffer>();
std::unique_ptr<PCHStorage> Storage;
if (StoreInMemory) {
- Storage = PCHStorage::inMemory();
+ Storage = PCHStorage::inMemory(Buffer);
} else {
// Create a temporary file for the precompiled preamble. In rare
// circumstances, this can fail.
@@ -495,9 +487,10 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
PreambleInputBuffer.release());
}
- std::unique_ptr<PrecompilePreambleAction> Act;
- Act.reset(new PrecompilePreambleAction(
- StoreInMemory ? &Storage->memoryBufferForWrite() : nullptr, Callbacks));
+ auto Act = std::make_unique<PrecompilePreambleAction>(
+ std::move(Buffer),
+ /*WritePCHFile=*/Storage->getKind() == PCHStorage::Kind::TempFile,
+ Callbacks);
if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
return BuildPreambleError::BeginSourceFileFailed;
@@ -527,6 +520,7 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
if (!Act->hasEmittedPreamblePCH())
return BuildPreambleError::CouldntEmitPCH;
+ Act.reset(); // Frees the PCH buffer frees, unless Storage keeps it in memory.
// Keep track of all of the files that the source manager knows about,
// so we can verify whether they have changed or not.
@@ -790,7 +784,8 @@ void PrecompiledPreamble::setupPreambleStorage(
StringRef PCHPath = getInMemoryPreamblePath();
PreprocessorOpts.ImplicitPCHInclude = std::string(PCHPath);
- auto Buf = llvm::MemoryBuffer::getMemBuffer(Storage.memoryContents());
+ auto Buf = llvm::MemoryBuffer::getMemBuffer(
+ Storage.memoryContents(), PCHPath, /*RequiresNullTerminator=*/false);
VFS = createVFSOverlayForPreamblePCH(PCHPath, std::move(Buf), VFS);
}
}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index aa9b4f149011b..74cc5c73c8031 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5071,7 +5071,8 @@ std::string ASTReader::getOriginalSourceFile(
const std::string &ASTFileName, FileManager &FileMgr,
const PCHContainerReader &PCHContainerRdr, DiagnosticsEngine &Diags) {
// Open the AST file.
- auto Buffer = FileMgr.getBufferForFile(ASTFileName);
+ auto Buffer = FileMgr.getBufferForFile(ASTFileName, /*IsVolatile=*/false,
+ /*RequiresNullTerminator=*/false);
if (!Buffer) {
Diags.Report(diag::err_fe_unable_to_read_pch_file)
<< ASTFileName << Buffer.getError().message();
More information about the cfe-commits
mailing list