[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#3014990 , @martong wrote: > @steakhal Since then we have our fancy csa-testbench based jenkins job(s) to > do measurement even on huge projects. Do you think it would make sense to > give it another measure with that?

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

martong added a comment. Herald added a subscriber: manas. In D86295#2539431 , @steakhal wrote: > In D86295#2519851 , @ASDenysPetrov > wrote: > >> What about this change? Did you make more measurements? > > IMO it

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2519851 , @ASDenysPetrov wrote: > What about this change? Did you make more measurements? IMO it needs more justification and measurement to land it. If my measurement was correct then it would decrease the readabilit

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

ASDenysPetrov added a comment. What about this change? Did you make more measurements? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86295/new/ https://reviews.llvm.org/D86295 ___ cfe-commits mailing lis

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. Should I create more measurements? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86295/new/ https://reviews.llvm.org/D86295 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

ASDenysPetrov accepted this revision. ASDenysPetrov added a comment. This revision is now accepted and ready to land. The change LGTM,!. If it really can speed up performance or improve memory organization, let's load this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. I tried to run the benchmark on the small set of projects but it would take an enormous time to analyze all such projects 20 times each... Dedicating a box for this is unfeasible for me. So I stuck with analyzing only `tmux` with 20 iterations. The results are not convin

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2233694 , @vsavchenko wrote: > And what is the error right now? F12760388: error.txt BTW this sort of ping-pong should be done on a different forum, eg. on the Static Analyzer Disc

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2233666 , @steakhal wrote: > In D86295#2233401 , @vsavchenko > wrote: > >> Yep, I guess that is the cause. I'll take a look. Did you try it with this >> fast fix? > > I trie

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2233401 , @vsavchenko wrote: > Yep, I guess that is the cause. I'll take a look. Did you try it with this > fast fix? I tried, but it lacks further fixes. Currently, I have this: diff --git a/clang/utils/analyzer

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2233398 , @steakhal wrote: > In D86295#2233244 , @vsavchenko > wrote: > >> Here is a short summary how to do regression testing (check that all >> warnings are the same): > >

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2233244 , @vsavchenko wrote: > Here is a short summary how to do regression testing (check that all warnings > are the same): Oh thanks for the detailed guide, I will make the experiment. However the `./SATest.py dock

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

Szelethus added a comment. I really like the patch, but have nothing to add to what other reviewers already said. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86295/new/ https://reviews.llvm.org/D86295 _

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2233157 , @steakhal wrote: > In D86295#2233076 , @vsavchenko > wrote: > >> In D86295#2231760 , @NoQ wrote: >> >>> I believe @vsavchenko

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2233076 , @vsavchenko wrote: > In D86295#2231760 , @NoQ wrote: > >> I believe @vsavchenko's docker thingy already knows how to do that. > > Yep, it sure does! Additionally, there

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2231760 , @NoQ wrote: > I believe @vsavchenko's docker thingy already knows how to do that. Yep, it sure does! Additionally, there is a `benchmark` subcommand that can chart memory consumption for measured projects.

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

martong added a comment. In D86295#2233068 , @martong wrote: > In D86295#2232005 , @steakhal wrote: > >> In D86295#2231760 , @NoQ wrote: >> >>> I mean, like, you can measure

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

martong added a comment. In D86295#2232005 , @steakhal wrote: > In D86295#2231760 , @NoQ wrote: > >> I mean, like, you can measure the entire process with `time` or something >> like that. I believe @vsavchenko's d

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2231760 , @NoQ wrote: > I mean, like, you can measure the entire process with `time` or something > like that. I believe @vsavchenko's docker thingy already knows how to do that. I tried to measure this with `time`, wi

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

NoQ added a comment. In D86295#2230603 , @steakhal wrote: > In D86295#2229712 , @NoQ wrote: > >> Heh, nice! Did you try to measure the actual impact of this change on memory >> and/or performance? > > Eh, I don't r

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2229712 , @NoQ wrote: > Heh, nice! Did you try to measure the actual impact of this change on memory > and/or performance? Eh, I don't really know how to bench this. Here is how I did it in nutshell: Added STATISTIC co

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

NoQ added a comment. Heh, nice! Did you try to measure the actual impact of this change on memory and/or performance? I think it'd make perfect sense to keep the offset in a side map. We don't compute it for all regions, and for most regions it doesn't need to be computed *at all*. Repositor

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2228745 , @xazax.hun wrote: > But unless you add a comment one will probably replace the offset with an > optional as the part of a refactoring. Sure, I will leave a note. I was thinking using a PointerIntUnion though

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

xazax.hun added a comment. MemRegion is a popular class to instantiate in the analyzer so it looks good to me. But unless you add a comment one will probably replace the offset with an optional as the part of a refactoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt