ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric, javed.absar.
To avoid doing extra work of processing headers in the preamble
mutilple times in parallel.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48940
Files:
clangd/TUScheduler.cpp
clangd/TUScheduler.h
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -101,6 +101,10 @@
/// - validate that the preamble is still valid, and only use it in this case
/// - accept that preamble contents may be outdated, and try to avoid reading
/// source code from headers.
+ /// However, Action will be scheduled to run after the first rebuild of the
+ /// preamble for the corresponding file finishes. Note that the rebuild can
+ /// still fail, in which case Action will get a null pointer instead of the
+ /// preamble.
/// If an error occurs during processing, it is forwarded to the \p Action
/// callback.
void runWithPreamble(llvm::StringRef Name, PathRef File,
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -176,6 +176,12 @@
bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
+ /// Wait for the first build of preamble to finish. Preamble itself can be
+ /// accessed via getPossibleStalePreamble(). Note that this function will
+ /// return after an unsuccessful build of the preamble too, i.e. result of
+ /// getPossiblyStalePreamble() can be null even after this function returns.
+ void waitForFirstPreamble() const;
+
std::size_t getUsedBytes() const;
bool isASTCached() const;
@@ -226,6 +232,8 @@
/// Guards members used by both TUScheduler and the worker thread.
mutable std::mutex Mutex;
std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex)
*/
+ /// Becomes ready when the first preamble build finishes.
+ Notification PreambleWasBuilt;
/// Set to true to signal run() to finish processing.
bool Done; /* GUARDED_BY(Mutex) */
std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
@@ -329,6 +337,9 @@
buildCompilerInvocation(Inputs);
if (!Invocation) {
log("Could not build CompilerInvocation for file " + FileName);
+ // Make sure anyone waiting for the preamble gets notified it could not
+ // be built.
+ PreambleWasBuilt.notify();
return;
}
@@ -340,6 +351,8 @@
if (NewPreamble)
LastBuiltPreamble = NewPreamble;
}
+ PreambleWasBuilt.notify();
+
// Build the AST for diagnostics.
llvm::Optional<ParsedAST> AST =
buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
@@ -392,6 +405,10 @@
return LastBuiltPreamble;
}
+void ASTWorker::waitForFirstPreamble() const {
+ PreambleWasBuilt.wait();
+}
+
std::size_t ASTWorker::getUsedBytes() const {
// Note that we don't report the size of ASTs currently used for processing
// the in-flight requests. We used this information for debugging purposes
@@ -655,6 +672,11 @@
std::string Contents,
tooling::CompileCommand Command, Context Ctx,
decltype(Action) Action) mutable {
+ // We don't want to be running preamble actions before the preamble was
+ // built for the first time. This avoids extra work of processing the
+ // preamble headers in parallel multiple times.
+ Worker->waitForFirstPreamble();
+
std::lock_guard<Semaphore> BarrierLock(Barrier);
WithContext Guard(std::move(Ctx));
trace::Span Tracer(Name);
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -101,6 +101,10 @@
/// - validate that the preamble is still valid, and only use it in this case
/// - accept that preamble contents may be outdated, and try to avoid reading
/// source code from headers.
+ /// However, Action will be scheduled to run after the first rebuild of the
+ /// preamble for the corresponding file finishes. Note that the rebuild can
+ /// still fail, in which case Action will get a null pointer instead of the
+ /// preamble.
/// If an error occurs during processing, it is forwarded to the \p Action
/// callback.
void runWithPreamble(llvm::StringRef Name, PathRef File,
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -176,6 +176,12 @@
bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
+ /// Wait for the first build of preamble to finish. Preamble itself can be
+ /// accessed via getPossibleStalePreamble(). Note that this function will
+ /// return after an unsuccessful build of the preamble too, i.e. result of
+ /// getPossiblyStalePreamble() can be null even after this function returns.
+ void waitForFirstPreamble() const;
+
std::size_t getUsedBytes() const;
bool isASTCached() const;
@@ -226,6 +232,8 @@
/// Guards members used by both TUScheduler and the worker thread.
mutable std::mutex Mutex;
std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
+ /// Becomes ready when the first preamble build finishes.
+ Notification PreambleWasBuilt;
/// Set to true to signal run() to finish processing.
bool Done; /* GUARDED_BY(Mutex) */
std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
@@ -329,6 +337,9 @@
buildCompilerInvocation(Inputs);
if (!Invocation) {
log("Could not build CompilerInvocation for file " + FileName);
+ // Make sure anyone waiting for the preamble gets notified it could not
+ // be built.
+ PreambleWasBuilt.notify();
return;
}
@@ -340,6 +351,8 @@
if (NewPreamble)
LastBuiltPreamble = NewPreamble;
}
+ PreambleWasBuilt.notify();
+
// Build the AST for diagnostics.
llvm::Optional<ParsedAST> AST =
buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
@@ -392,6 +405,10 @@
return LastBuiltPreamble;
}
+void ASTWorker::waitForFirstPreamble() const {
+ PreambleWasBuilt.wait();
+}
+
std::size_t ASTWorker::getUsedBytes() const {
// Note that we don't report the size of ASTs currently used for processing
// the in-flight requests. We used this information for debugging purposes
@@ -655,6 +672,11 @@
std::string Contents,
tooling::CompileCommand Command, Context Ctx,
decltype(Action) Action) mutable {
+ // We don't want to be running preamble actions before the preamble was
+ // built for the first time. This avoids extra work of processing the
+ // preamble headers in parallel multiple times.
+ Worker->waitForFirstPreamble();
+
std::lock_guard<Semaphore> BarrierLock(Barrier);
WithContext Guard(std::move(Ctx));
trace::Span Tracer(Name);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
