erichkeane wrote: > Looks like the regression was caused by `areAllIgnored`. It is really slow. > > Even after I tried some optimizations for the function, I can't restore the > original improvements. (Looking the group up by `diag::Group` instead of by > name, giving the `SmallVector` enough inline capacity to avoid a malloc per > call, and stopping at the first enabled diagnostic instead of computing the > highest severity of the whole group.) and i am still at +.15% regression > > [11d4c47](https://github.com/llvm/llvm-project/commit/11d4c47f19a7cae3dfd86ef29a3900c054ac6553) > > ```c++ > bool areAllIgnored(StringRef Group, SourceLocation Loc) const { > llvm::SmallVector<diag::kind> diagsInGroup; > bool Failed = Diags->getDiagnosticsInGroup(diag::Flavor::WarningOrError, > Group, diagsInGroup); > assert(!Failed && "Incorrect group name?"); > (void)Failed; > return Diags->getDiagnosticListHighestSeverity(diagsInGroup, Loc, *this) > == > diag::Severity::Ignored; > } > ``` > > CC @AaronBallman @erichkeane Any advice?
Wow, 3-10% hit is huge. But I guess if it is the first thing we're doing on ANY comment, that is going to happen a lot. That said, I'd expect this to become 'performance positive'. Since you are checking 2 groups at once, I DO suspect there is value in making the signature `bool areAllIgnored(ArrayRef<StringRef> Groups, SourceLocation Loc) const` kinda thing (perhaps halving the cost?). Another option would be a higher touch attempt to make an alternative to `getDiagnosticListHighestSeverity` that does an 'exit' early on `first non-ignored` instead of `first max size`. BUT the real solution here is perhaps to have Sema (OR the diagnostics engine?!) `cache` the value. You'd have to invalidate that value whenever we saw one of the `warning group invalidating` operations though. `Sema` caching would be a little more error prone, but faster (Since you'd have to make sure any time we see one of the in-source changes to pragmas, you'd have to clear your cache). In the diagnostics-engine, we only have a handful of modifying functions (`setSeverity`, setSeverityForGroup`, `setSeverityForAll`, `Reset`, and `ResetPragmas` maybe?) that you could either find all uses of in Sema, or just have invalidate a `DiagnosticsEngine` cache. For a cache in sema, I'd expect just a `std::optional<bool> documentationAndDocPedanticIgnored` type thing. For DiagnosticsEngine, I wonder if we should cache ANY entry into `areAllIgnored`, so a DenseMap of string/group to result. https://github.com/llvm/llvm-project/pull/206363 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
