[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-30 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. > So I think the most valuable optimizations are low-level optimizations to > `ImmutableMap`. There were a few suggestions on the mailing list to use > something more modern than the AVL trees under the hood but I don't think > authors found much success with those.

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D131707#3724747 , @NoQ wrote: > So I think the most valuable optimizations are low-level optimizations to > `ImmutableMap`. There were a few suggestions on the mailing list to use > something more modern than the AVL trees

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Hmm at a glance I disagree with the FIXME, this doesn't look like a valuable optimization. This operation is typically constant-time anyway so it's probably not worth the memory impact and the added complexity. The easiest way to roughly estimate impact is to take

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a subscriber: balazske. steakhal added a comment. In D131707#3723403 , @ASDenysPetrov wrote: > @steakhal > Thank you for the review. > Could you please do testing for me, since I'm on Windows and have no prepared > testing environment as

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. In D131707#3723403 , @ASDenysPetrov wrote: > @steakhal > Thank you for the review. > Could you please do testing for me, since I'm on Windows and have no prepared > testing environment as I know you do. > I'll add an

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 452689. ASDenysPetrov added a comment. Add assertion accroding to the remark. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131707/new/ https://reviews.llvm.org/D131707 Files:

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @steakhal Thank you for the review. Could you please do testing for me, since I'm on Windows and have no prepared testing environment as I know you are. I'll add an assertion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

2022-08-15 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Please measure the performance implications/gains of this patch to justify this change. Preferably test it on multiple kinds of projects ranging from old-school C and even some modern C++ projects. Let me know your findings and if you need some help setting it up.

[PATCH] D131707: [analyzer]{NFC} Cache the result of getLocationType in TypedValueRegion

2022-08-11 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov created this revision. ASDenysPetrov added reviewers: steakhal, martong, NoQ, xazax.hun, isuckatcs. ASDenysPetrov added a project: clang. Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Herald added a