Skip to content

Commit 6c465a2

Browse files
jansvoboda11jansvoboda11
authored
[clang][deps] Skip slow UNHASHED_CONTROL_BLOCK records (#69975)
Deserialization of the `DIAGNOSTIC_OPTIONS` and `HEADER_SEARCH_PATHS` records is slow and done for every transitively loaded PCM. Deserialization of these records cannot be skipped, because the words are VBR6-encoded and we don't store the length of the entire record. We could either turn them into binary blobs that can be skipped during deserialization, or skip writing them altogether. This patch takes the latter approach, since these records are not necessary in scanning PCMs. The scanner doesn't make any guarantees about the accuracy of diagnostics, and we always have the same header search paths due to strict context hashing. The commit that makes the `DIAGNOSTIC_OPTIONS` record skippable was originally implemented by @benlangmuir in a downstream repo.
1 parent 8750239 commit 6c465a2

File tree

7 files changed

+149
-35
lines changed

7 files changed

+149
-35
lines changed

‎clang/include/clang/Driver/Options.td

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2947,6 +2947,14 @@ def fno_modules_validate_textual_header_includes :
29472947
MarshallingInfoNegativeFlag<LangOpts<"ModulesValidateTextualHeaderIncludes">>,
29482948
HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. "
29492949
"This flag will be removed in a future Clang release.">;
2950+
defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options",
2951+
HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse,
2952+
PosFlag<SetTrue, [], [], "Disable writing diagnostic options">,
2953+
NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>;
2954+
defm modules_skip_header_search_paths : BoolFOption<"modules-skip-header-search-paths",
2955+
HeaderSearchOpts<"ModulesSkipHeaderSearchPaths">, DefaultFalse,
2956+
PosFlag<SetTrue, [], [], "Disable writing header search paths">,
2957+
NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>;
29502958

29512959
def fincremental_extensions :
29522960
Flag<["-"], "fincremental-extensions">,

‎clang/include/clang/Lex/HeaderSearchOptions.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,14 @@ class HeaderSearchOptions {
219219

220220
unsigned ModulesValidateDiagnosticOptions : 1;
221221

222+
/// Whether to entirely skip writing diagnostic options.
223+
/// Primarily used to speed up deserialization during dependency scanning.
224+
unsigned ModulesSkipDiagnosticOptions : 1;
225+
226+
/// Whether to entirely skip writing header search paths.
227+
/// Primarily used to speed up deserialization during dependency scanning.
228+
unsigned ModulesSkipHeaderSearchPaths : 1;
229+
222230
unsigned ModulesHashContent : 1;
223231

224232
/// Whether we should include all things that could impact the module in the
@@ -238,7 +246,9 @@ class HeaderSearchOptions {
238246
ModulesValidateSystemHeaders(false),
239247
ValidateASTInputFilesContent(false),
240248
ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false),
241-
ModulesValidateDiagnosticOptions(true), ModulesHashContent(false),
249+
ModulesValidateDiagnosticOptions(true),
250+
ModulesSkipDiagnosticOptions(false),
251+
ModulesSkipHeaderSearchPaths(false), ModulesHashContent(false),
242252
ModulesStrictContextHash(false) {}
243253

244254
/// AddPath - Add the \p Path path to the specified \p Group list.

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,21 @@ namespace {
661661
return false;
662662
}
663663

664+
bool ReadHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
665+
bool Complain) override {
666+
Out.indent(2) << "Header search paths:\n";
667+
Out.indent(4) << "User entries:\n";
668+
for (const auto &Entry : HSOpts.UserEntries)
669+
Out.indent(6) << Entry.Path << "\n";
670+
Out.indent(4) << "System header prefixes:\n";
671+
for (const auto &Prefix : HSOpts.SystemHeaderPrefixes)
672+
Out.indent(6) << Prefix.Prefix << "\n";
673+
Out.indent(4) << "VFS overlay files:\n";
674+
for (const auto &Overlay : HSOpts.VFSOverlayFiles)
675+
Out.indent(6) << Overlay << "\n";
676+
return false;
677+
}
678+
664679
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
665680
bool ReadMacros, bool Complain,
666681
std::string &SuggestedPredefines) override {

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,52 +1212,54 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
12121212
Record.clear();
12131213
}
12141214

1215+
const auto &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
1216+
12151217
// Diagnostic options.
12161218
const auto &Diags = Context.getDiagnostics();
12171219
const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
1220+
if (!HSOpts.ModulesSkipDiagnosticOptions) {
12181221
#define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name);
12191222
#define ENUM_DIAGOPT(Name, Type, Bits, Default) \
12201223
Record.push_back(static_cast<unsigned>(DiagOpts.get##Name()));
12211224
#include "clang/Basic/DiagnosticOptions.def"
1222-
Record.push_back(DiagOpts.Warnings.size());
1223-
for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
1224-
AddString(DiagOpts.Warnings[I], Record);
1225-
Record.push_back(DiagOpts.Remarks.size());
1226-
for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
1227-
AddString(DiagOpts.Remarks[I], Record);
1228-
// Note: we don't serialize the log or serialization file names, because they
1229-
// are generally transient files and will almost always be overridden.
1230-
Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
1231-
Record.clear();
1225+
Record.push_back(DiagOpts.Warnings.size());
1226+
for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
1227+
AddString(DiagOpts.Warnings[I], Record);
1228+
Record.push_back(DiagOpts.Remarks.size());
1229+
for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
1230+
AddString(DiagOpts.Remarks[I], Record);
1231+
// Note: we don't serialize the log or serialization file names, because
1232+
// they are generally transient files and will almost always be overridden.
1233+
Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
1234+
Record.clear();
1235+
}
12321236

12331237
// Header search paths.
1234-
Record.clear();
1235-
const HeaderSearchOptions &HSOpts =
1236-
PP.getHeaderSearchInfo().getHeaderSearchOpts();
1237-
1238-
// Include entries.
1239-
Record.push_back(HSOpts.UserEntries.size());
1240-
for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
1241-
const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
1242-
AddString(Entry.Path, Record);
1243-
Record.push_back(static_cast<unsigned>(Entry.Group));
1244-
Record.push_back(Entry.IsFramework);
1245-
Record.push_back(Entry.IgnoreSysRoot);
1246-
}
1238+
if (!HSOpts.ModulesSkipHeaderSearchPaths) {
1239+
// Include entries.
1240+
Record.push_back(HSOpts.UserEntries.size());
1241+
for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
1242+
const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
1243+
AddString(Entry.Path, Record);
1244+
Record.push_back(static_cast<unsigned>(Entry.Group));
1245+
Record.push_back(Entry.IsFramework);
1246+
Record.push_back(Entry.IgnoreSysRoot);
1247+
}
12471248

1248-
// System header prefixes.
1249-
Record.push_back(HSOpts.SystemHeaderPrefixes.size());
1250-
for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
1251-
AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
1252-
Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
1253-
}
1249+
// System header prefixes.
1250+
Record.push_back(HSOpts.SystemHeaderPrefixes.size());
1251+
for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
1252+
AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
1253+
Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
1254+
}
12541255

1255-
// VFS overlay files.
1256-
Record.push_back(HSOpts.VFSOverlayFiles.size());
1257-
for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles)
1258-
AddString(VFSOverlayFile, Record);
1256+
// VFS overlay files.
1257+
Record.push_back(HSOpts.VFSOverlayFiles.size());
1258+
for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles)
1259+
AddString(VFSOverlayFile, Record);
12591260

1260-
Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
1261+
Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
1262+
}
12611263

12621264
// Write out the diagnostic/pragma mappings.
12631265
WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule);

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ class DependencyScanningAction : public tooling::ToolAction {
252252
// TODO: Implement diagnostic bucketing to reduce the impact of strict
253253
// context hashing.
254254
ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
255+
ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
256+
ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
255257

256258
// Avoid some checks and module map parsing when loading PCM files.
257259
ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;

‎clang/test/Modules/diagnostic-options-mismatch.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: rm -rf %t && mkdir -p %t
2+
// RUN: split-file %s %t
3+
4+
//--- module.modulemap
5+
module Mod { header "mod.h" }
6+
//--- mod.h
7+
//--- tu.c
8+
#include "mod.h"
9+
10+
// Without any extra compiler flags, mismatched diagnostic options trigger recompilation of modules.
11+
//
12+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
13+
// RUN: -fsyntax-only %t/tu.c -Wnon-modular-include-in-module
14+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
15+
// RUN: -fsyntax-only %t/tu.c -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
16+
// RUN: | FileCheck %s --check-prefix=DID-REBUILD
17+
// DID-REBUILD: remark: building module 'Mod'
18+
19+
// When skipping serialization of diagnostic options, mismatches cannot be detected, old PCM file gets reused.
20+
//
21+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
22+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Wnon-modular-include-in-module
23+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
24+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
25+
// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
26+
// DID-REUSE-NOT: remark: building module 'Mod'
27+
//
28+
// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-DIAG-OPTS
29+
// NO-DIAG-OPTS-NOT: Diagnostic flags:
30+
31+
// When disabling validation of diagnostic options, mismatches are not checked for, old PCM file gets reused.
32+
//
33+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \
34+
// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Wnon-modular-include-in-module
35+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \
36+
// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
37+
// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
38+
//
39+
// RUN: %clang_cc1 -module-file-info %t/cache3/Mod.pcm | FileCheck %s --check-prefix=OLD-DIAG-OPTS
40+
// OLD-DIAG-OPTS: Diagnostic flags:
41+
// OLD-DIAG-OPTS-NEXT: -Wnon-modular-include-in-module

‎clang/test/Modules/header-search-paths-mismatch.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: rm -rf %t && mkdir -p %t
2+
// RUN: split-file %s %t
3+
4+
//--- module.modulemap
5+
module Mod { header "mod.h" }
6+
//--- mod.h
7+
//--- tu.c
8+
#include "mod.h"
9+
10+
//--- one/foo.h
11+
//--- two/foo.h
12+
13+
// By default, mismatched header search paths are ignored, old PCM file gets reused.
14+
//
15+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
16+
// RUN: -fsyntax-only %t/tu.c -I %t/one
17+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
18+
// RUN: -fsyntax-only %t/tu.c -I %t/two -Rmodule-build 2>&1 \
19+
// RUN: | FileCheck %s --allow-empty --check-prefix=DID-REUSE
20+
// DID-REUSE-NOT: remark: building module 'Mod'
21+
//
22+
// RUN: %clang_cc1 -module-file-info %t/cache1/Mod.pcm | FileCheck %s --check-prefix=HS-PATHS
23+
// HS-PATHS: Header search paths:
24+
// HS-PATHS-NEXT: User entries:
25+
// HS-PATHS-NEXT: one
26+
27+
// When skipping serialization of header search paths, mismatches cannot be detected, old PCM file gets reused.
28+
//
29+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
30+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-header-search-paths -I %t/one
31+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
32+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-header-search-paths -I %t/two -Rmodule-build 2>&1 \
33+
// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
34+
//
35+
// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-HS-PATHS
36+
// NO-HS-PATHS-NOT: Header search paths:

0 commit comments

Comments
 (0)