Page MenuHomePhabricator

[clang] Optimize storage and lookup of analyzer options
Changes PlannedPublic

Authored by jansvoboda11 on Nov 2 2022, 8:26 AM.

Details

Summary

This patch removes std::vector, std::sort() and std::binary_search() in AnalyzerOptions with a static llvm::StringSwitch.

This avoids unnecessary work, which can speed up Clang tools that initialize lots of CompilerInvocations (and therefore AnalyzerOptions).

Diff Detail

Event Timeline

jansvoboda11 created this revision.Nov 2 2022, 8:26 AM
Herald added a project: Restricted Project. · View Herald Transcript
jansvoboda11 requested review of this revision.Nov 2 2022, 8:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2022, 8:26 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Are you sure about the speedup?
When I looked at the implementation of the StringSwitch, it boiled down to an if-else chain under the hood.

steakhal accepted this revision.Nov 24 2022, 7:27 AM

BTW the change itself looks good to me.

This revision is now accepted and ready to land.Nov 24 2022, 7:27 AM
jansvoboda11 planned changes to this revision.Dec 2 2022, 10:57 AM

Are you sure about the speedup?
When I looked at the implementation of the StringSwitch, it boiled down to an if-else chain under the hood.

You're right. I assumed StringSwitch was more sophisticated than that. This patch provides speedup for my use-case, which involves constructing lots of AnalyzerOptions instances, but no calls to AnalyzerOptions::isUnknownAnalyzerConfig(). I'll spend more time on this and make sure I don't regress the lookup. (Unfortunately, std::sort is only constexpr in C++20 onwards, so I can't just sort the options at compile-time.)