Skip to content

Commit 904b4f5

Browse files
authored
[clang][timers][modules] Fix a timer being started when it's running (#154231)
`ASTReader::FinishedDeserializing()` calls `adjustDeductedFunctionResultType(...)` [0], which in turn calls `FunctionDecl::getMostRecentDecl()`[1]. In modules builds, `getMostRecentDecl()` may reach out to the `ASTReader` and start deserializing again. Starting deserialization starts `ReadTimer`; however, `FinishedDeserializing()` doesn't call `stopTimer()` until after it's call to `adjustDeductedFunctionResultType(...)` [2]. As a result, we hit an assert checking that we don't start an already started timer [3]. To fix this, we simply don't start the timer if it's already running. Unfortunately I don't have a test case for this yet as modules builds are notoriously difficult to reduce. [0]: https://github.com/llvm/llvm-project/blob/4d2288d31866013bd361800df4746fbc2fcf742a/clang/lib/Serialization/ASTReader.cpp#L11053 [1]: https://github.com/llvm/llvm-project/blob/4d2288d31866013bd361800df4746fbc2fcf742a/clang/lib/AST/ASTContext.cpp#L3804 [2]: https://github.com/llvm/llvm-project/blob/4d2288d31866013bd361800df4746fbc2fcf742a/clang/lib/Serialization/ASTReader.cpp#L11065-L11066 [3]: https://github.com/llvm/llvm-project/blob/4d2288d31866013bd361800df4746fbc2fcf742a/llvm/lib/Support/Timer.cpp#L150
1 parent 55551da commit 904b4f5

File tree

2 files changed

+7
-4
lines changed

2 files changed

+7
-4
lines changed

clang/include/clang/Serialization/ASTReader.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,9 @@ class ASTReader
526526
/// A timer used to track the time spent deserializing.
527527
std::unique_ptr<llvm::Timer> ReadTimer;
528528

529+
// A TimeRegion used to start and stop ReadTimer via RAII.
530+
std::optional<llvm::TimeRegion> ReadTimeRegion;
531+
529532
/// The location where the module file will be considered as
530533
/// imported from. For non-module AST types it should be invalid.
531534
SourceLocation CurrentImportLoc;

clang/lib/Serialization/ASTReader.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11008,8 +11008,9 @@ void ASTReader::diagnoseOdrViolations() {
1100811008
}
1100911009

1101011010
void ASTReader::StartedDeserializing() {
11011-
if (++NumCurrentElementsDeserializing == 1 && ReadTimer.get())
11012-
ReadTimer->startTimer();
11011+
if (llvm::Timer *T = ReadTimer.get();
11012+
++NumCurrentElementsDeserializing == 1 && T)
11013+
ReadTimeRegion.emplace(T);
1101311014
}
1101411015

1101511016
void ASTReader::FinishedDeserializing() {
@@ -11067,8 +11068,7 @@ void ASTReader::FinishedDeserializing() {
1106711068
(void)UndeducedFD->getMostRecentDecl();
1106811069
}
1106911070

11070-
if (ReadTimer)
11071-
ReadTimer->stopTimer();
11071+
ReadTimeRegion.reset();
1107211072

1107311073
diagnoseOdrViolations();
1107411074
}

0 commit comments

Comments
 (0)