sepavloff created this revision.
sepavloff added a subscriber: cfe-commits.
This change fixes performance degradation reported in PR24667. The reason
of the problem is leaving module after each header, even if the next header
is of the same module. With this fix module state record counts the number of
headers parsed within particular module. Macro update starts when this number
reaches the number of headers specified in the module descriptor.
Using script from PR24667 the performance measurement gives numbers (compile
time vs number of headers):
128 0.0362679958344
256 0.0517950057983
512 0.0877501964569
1024 0.171896934509
2048 0.393149137497
4096 1.02584791183
8192 3.1531829834
While without the fix compiler demonstrates quadratic compile time:
128 0.0541491508484
256 0.121176958084
512 0.330996990204
1024 1.10708594322
2048 4.06581497192
4096 16.0358121395
8192 66.6967120171
http://reviews.llvm.org/D13987
Files:
include/clang/Lex/Preprocessor.h
lib/Lex/PPLexerChange.cpp
Index: lib/Lex/PPLexerChange.cpp
===================================================================
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -404,6 +404,8 @@
bool LeavingSubmodule = CurSubmodule && CurLexer;
if (LeavingSubmodule) {
+ ++CurSubmoduleState->NumCompiledHeaders;
+
// Notify the parser that we've left the module.
const char *EndPos = getCurLexerEndPos();
Result.startToken();
@@ -701,42 +703,47 @@
// This module may have exported a new macro. If so, create a ModuleMacro
// representing that fact.
- bool ExplicitlyPublic = false;
- for (auto *MD = Macro.second.getLatest(); MD != OldMD;
- MD = MD->getPrevious()) {
- assert(MD && "broken macro directive chain");
-
- // Stop on macros defined in other submodules we #included along the way.
- // There's no point doing this if we're tracking local submodule
- // visibility, since there can be no such directives in our list.
- if (!getLangOpts().ModulesLocalVisibility) {
- Module *Mod = getModuleContainingLocation(MD->getLocation());
- if (Mod != LeavingMod)
- break;
- }
+ // We do this only if when the last module header is compiled, to avoid
+ // repeating updates of macro set.
+ if (CurSubmoduleState->NumCompiledHeaders >=
+ LeavingMod->Headers[Module::HK_Normal].size()) {
+ bool ExplicitlyPublic = false;
+ for (auto *MD = Macro.second.getLatest(); MD != OldMD;
+ MD = MD->getPrevious()) {
+ assert(MD && "broken macro directive chain");
+
+ // Stop on macros defined in other submodules we #included along the
+ // way. There's no point doing this if we're tracking local submodule
+ // visibility, since there can be no such directives in our list.
+ if (!getLangOpts().ModulesLocalVisibility) {
+ Module *Mod = getModuleContainingLocation(MD->getLocation());
+ if (Mod != LeavingMod)
+ break;
+ }
- if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
- // The latest visibility directive for a name in a submodule affects
- // all the directives that come before it.
- if (VisMD->isPublic())
- ExplicitlyPublic = true;
- else if (!ExplicitlyPublic)
- // Private with no following public directive: not exported.
+ if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
+ // The latest visibility directive for a name in a submodule affects
+ // all the directives that come before it.
+ if (VisMD->isPublic())
+ ExplicitlyPublic = true;
+ else if (!ExplicitlyPublic)
+ // Private with no following public directive: not exported.
+ break;
+ } else {
+ MacroInfo *Def = nullptr;
+ if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))
+ Def = DefMD->getInfo();
+
+ // FIXME: Issue a warning if multiple headers for the same submodule
+ // define a macro, rather than silently ignoring all but the first.
+ bool IsNew;
+ // Don't bother creating a module macro if it would represent a #undef
+ // that doesn't override anything.
+ if (Def || !Macro.second.getOverriddenMacros().empty())
+ addModuleMacro(LeavingMod, II, Def,
+ Macro.second.getOverriddenMacros(), IsNew);
break;
- } else {
- MacroInfo *Def = nullptr;
- if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))
- Def = DefMD->getInfo();
-
- // FIXME: Issue a warning if multiple headers for the same submodule
- // define a macro, rather than silently ignoring all but the first.
- bool IsNew;
- // Don't bother creating a module macro if it would represent a #undef
- // that doesn't override anything.
- if (Def || !Macro.second.getOverriddenMacros().empty())
- addModuleMacro(LeavingMod, II, Def,
- Macro.second.getOverriddenMacros(), IsNew);
- break;
+ }
}
}
}
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -522,10 +522,13 @@
/// \brief Information about a submodule's preprocessor state.
struct SubmoduleState {
+ SubmoduleState() : NumCompiledHeaders(0) {}
/// The macros for the submodule.
MacroMap Macros;
/// The set of modules that are visible within the submodule.
VisibleModuleSet VisibleModules;
+ /// Number of module headers compiled so far.
+ unsigned NumCompiledHeaders;
// FIXME: CounterValue?
// FIXME: PragmaPushMacroInfo?
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
