bcraig closed this revision.
bcraig added a comment.
Completed: At revision: 272394
http://reviews.llvm.org/D20933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
Ah, right, please, add a comment explaining what we are doing and why.
http://reviews.llvm.org/D20933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
http://reviews.llvm.org/D20933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
bcraig added a comment.
I got better valgrind numbers (symbols are important).
Before: 106,131,538
After: 106,657,666
Diff: 526,128 larger.
Note that this is sampled peaks for heap usage. They may not be accurate.
Regardless, the peak usage increased by less than .5%.
bcraig updated this revision to Diff 60177.
bcraig added a comment.
Capping the pre-reserve space
http://reviews.llvm.org/D20933
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
lib/StaticAnalyzer/Core/CoreEngine.cpp
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
zaks.anna added a comment.
> If the underlying allocator that does a poor job at reusing freed memory,
> then trivial
> functions will use about 1 MB more than before, then free the memory
> immediately.
You could probably flag some of those functions, especially the ones that do
not
bcraig added a comment.
In http://reviews.llvm.org/D20933#452854, @dcoughlin wrote:
> A 6% speed improvement could be a big win! Do we have a sense of what the
> expected increased memory cost (as a % of average use over the lifetime of
> the process) is? My guess is it would be relatively
dcoughlin added a comment.
A 6% speed improvement could be a big win! Do we have a sense of what the
expected increased memory cost (as a % of average use over the lifetime of the
process) is? My guess is it would be relatively low. I suspect most analyzer
users run relatively few concurrent
bcraig added a comment.
In http://reviews.llvm.org/D20933#452352, @zaks.anna wrote:
> > For the pile of LLVM projects that I am currently building (llvm, clang,
> > libcxx, libcxxabi), 18.9% of all analyzed
>
> > functions hit the maximum step count. For the previously discussed large
> > .C
zaks.anna added a comment.
> For the pile of LLVM projects that I am currently building (llvm, clang,
> libcxx, libcxxabi), 18.9% of all analyzed
> functions hit the maximum step count. For the previously discussed large .C
> file, 37% of the analyzed functions hit the maximum step count.
bcraig updated this revision to Diff 59950.
bcraig added a comment.
Uploading more context.
http://reviews.llvm.org/D20933
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
lib/StaticAnalyzer/Core/CoreEngine.cpp
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
bcraig added a comment.
tldr; I'm going to recommend we stick with the current algorithm, but I only
have partial data to back that up.
For the pile of LLVM projects that I am currently building (llvm, clang,
libcxx, libcxxabi), 18.9% of all analyzed functions hit the maximum step count.
For
zaks.anna added a comment.
> Specifically, I suspect that C (and maybe ObjC?) won't hit the analysis limit
> often, but that C++ will hit the
> limit frequently because of the large number of inline functions.
It might be valuable to know this. Maybe we should changer the default for C++?
bcraig added a comment.
I'll try to get those stats. My suspicion is that it is highly language /
projects specific. Specifically, I suspect that C (and maybe ObjC?) won't hit
the analysis limit often, but that C++ will hit the limit frequently because of
the large number of inline
zaks.anna added a comment.
Ben,
By the way, thanks for the patch! It's a clever idea.
> The implementation of FoldingSet has a vector of bucket pointers. Reserve
> preallocates that vector of
> bucket pointers to the correct size to avoid rehashing. The allocation of
> the nodes themselves
bcraig added a comment.
In http://reviews.llvm.org/D20933#447608, @zaks.anna wrote:
> Does FoldingSet already have reserve? (I do not see it.)
The reserve call would be new. I'm attempting to add it in this llvm review:
http://reviews.llvm.org/D20930
> My understanding is that you propose
zaks.anna added a comment.
Does FoldingSet already have reserve? (I do not see it.)
My understanding is that you propose to allocate all the nodes that would be
required to store the maximum number of nodes we allow, lowering the malloc
traffic. The current implementation just doubles the
bcraig created this revision.
bcraig added reviewers: zaks.anna, krememek, jordan_rose.
bcraig added a subscriber: cfe-commits.
This depends on http://reviews.llvm.org/D20930
Rehashing the ExplodedNode table is very expensive. The hashing itself is
expensive, and the general activity of
18 matches
Mail list logo
