djasper added inline comments.
================ Comment at: lib/Basic/Diagnostic.cpp:179 + + // 2nd most frequent case: L is before the first diag state change. + FullSourceLoc FirstStateChangePos = DiagStatePoints[1].Loc; ---------------- rsmith wrote: > It's surprising to me that this would be particularly frequent. > > I suspect what you're actually seeing is a consequence of a bug in how we > manage `DiagStatePoint`s with modules. It looks like > `ASTReader::InitializeContext` is called once per top-level PCM file that we > load, and its call to `ReadPragmaDiagnosticMappings` adds entries to the > `DiagStatePoints` list regardless of whether they've already been added. So, > we'll end up with duplicates in the `DiagStatePoints` list, and it won't even > be in translation unit order. > > Can you take a look at the `DiagStatePoints` list for a translation unit > where you see a performance problem and check whether it seems reasonable? I looked at the DiagStatePoints and they do look somewhat sane but suspicious. The translation unit I am looking at has (AFAICT) only includes that get translated to modules. DiagStatePoints only has entries for the translation unit itself, not a single one coming from any of the modules. So DiagStatePoints looks like: [0]: <<invalid>> (for the command line) [1]: myfile.cc:100 [2]: myfile.cc:100 ... [2500]: myfile.cc:2800 And because of that, the new fast path is hit every single time when a source location coming from a module is queried. There are always two entries for a line of myfile.cc which always seem to denote entering and exiting a macro. So, specific questions: - Should there by DiagStatePoints from modules? - Should there really be a DiagStatePoint entry (or actually two) for every macro invocation in myfile.cc? https://reviews.llvm.org/D28207 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
