[PATCH] Inverse post-order traversal for LiveVariables analysis, to recover the performance after r214064

Sat Sep 20 17:36:21 PDT 2014

BTW, here's a relevant stack trace:

#0  llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*>
>::add_internal (this=0x173ca90, V=0x1a0db10, T=0x20d5610) at
llvm/include/llvm/ADT/ImmutableSet.h:551
#1  0x0000000000d76b25 in add_internal (T=0x244be90, V=<optimized out>,
this=0x173ca90) at llvm/include/llvm/ADT/ImmutableSet.h:549
#2  add_internal (T=0x22ef820, V=<optimized out>, this=0x173ca90) at
llvm/include/llvm/ADT/ImmutableSet.h:551
#3  add_internal (T=0x1f15790, V=<optimized out>, this=0x173ca90) at
llvm/include/llvm/ADT/ImmutableSet.h:551
#4  llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*>
>::add_internal (this=0x173ca90, V=<optimized out>, T=0x250a1b0)
    at llvm/include/llvm/ADT/ImmutableSet.h:551
#5  0x0000000000d768e1 in add_internal (T=0x20eb1d0, V=<optimized out>,
this=0x173ca90) at llvm/include/llvm/ADT/ImmutableSet.h:551
#6  add_internal (T=0x1f2c190, V=<optimized out>, this=0x173ca90) at
llvm/include/llvm/ADT/ImmutableSet.h:551
#7  add_internal (T=0x24ca520, V=<optimized out>, this=0x173ca90) at
llvm/include/llvm/ADT/ImmutableSet.h:551
#8  llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*>
>::add_internal (this=0x173ca90, V=<optimized out>, T=<optimized out>)
    at llvm/include/llvm/ADT/ImmutableSet.h:549
#9  0x0000000000d78bd2 in
llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add
(this=this at entry=0x173ca90, T=T at entry=0x2081090, V=<optimized out>)
    at llvm/include/llvm/ADT/ImmutableSet.h:401
#10 0x0000000000d7cca1 in add (V=<optimized out>, this=<optimized out>) at
llvm/include/llvm/ADT/ImmutableSet.h:1158
#11 mergeSets<llvm::ImmutableSetRef<clang::Stmt const*> > (B=..., A=...) at
llvm/tools/clang/lib/Analysis/LiveVariables.cpp:81
#12 merge (valsB=<error reading variable: access outside bounds of object
referenced via synthetic pointer>,
    valsA=<error reading variable: access outside bounds of object
referenced via synthetic pointer>, this=0x173ca70) at
llvm/tools/clang/lib/Analysis/LiveVariables.cpp:103
#13 clang::LiveVariables::computeLiveness (AC=...,
killAtAssign=killAtAssign at entry=true) at
llvm/tools/clang/lib/Analysis/LiveVariables.cpp:489
#14 0x0000000000619099 in create (analysisContext=...) at
llvm/tools/clang/include/clang/Analysis/Analyses/LiveVariables.h:99
#15 getAnalysis<clang::LiveVariables> (this=0x17db180) at
llvm/tools/clang/include/clang/Analysis/AnalysisContext.h:200
#16 getAnalysis<clang::LiveVariables> (D=0x197c7d0, this=0x173ac30) at
llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:119
#17 checkASTCodeBody (BR=..., mgr=..., D=0x197c7d0, this=<optimized out>)
at llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:437
#18 clang::ento::check::ASTCodeBody::_checkBody<(anonymous
namespace)::DeadStoresChecker> (checker=0x173bae0, D=0x197c7d0, mgr=...,
BR=...)
    at llvm/tools/clang/include/clang/StaticAnalyzer/Core/Checker.h:56
#19 0x00000000008c42b0 in operator() (p3=..., p2=..., p1=0x197c7d0,
this=<optimized out>) at
llvm/tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:82
#20 clang::ento::CheckerManager::runCheckersOnASTBody (this=0x173a930,
D=D at entry=0x197c7d0, mgr=..., BR=...) at
llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:86
#21 0x000000000052e41d in (anonymous
namespace)::AnalysisConsumer::HandleCode(clang::Decl *, (anonymous
namespace)::AnalysisConsumer::AnalysisMode, enum
clang::ento::ExprEngine::InliningModes, clang::ento::SetOfConstDecls *)
(this=this at entry=0x16dffa0, D=D at entry=0x197c7d0, Mode=1,
IMode=IMode at entry=clang::ento::ExprEngine::Inline_Minimal,
VisitedCallees=VisitedCallees at entry=0x0)
    at llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:623

On Sun, Sep 21, 2014 at 2:16 AM, Alexander Kornienko <alexfh at google.com>
wrote:

> I'm attaching one of the files that regressed after r214064 and never got
> fixed (preprocessed ceval.c from Python 3.3).
>
>
> On Sat, Sep 20, 2014 at 5:38 PM, Alexander Kornienko <alexfh at google.com>
> wrote:
>
>> I actually still think, that I have some code that started taking large
>> time to be analyzed after r214064 and didn't recover after r215650. But I
>> didn't get to creating a reasonable repro for you. And the number of files
>> left affected after r215650 is so small, that I didn't prioritize this high
>> enough. I'll still try to provide a repro soon.
>> On 20 Sep 2014 17:10, "Artyom Skrobov" <Artyom.Skrobov at arm.com> wrote:
>>
>>> Anna, do you mean the performance had been acceptable after r214064, but
>>> degraded after r215650, which fixed the performance regression introduced
>>> in r214064?
>>>
>>>
>>>
>>> Do you have any specific example of code that takes longer to compile
>>> after r215650?
>>>
>>>
>>>
>>> Not hearing back from Alexander since August, I assumed the performance
>>> regression he observed after r215650 was not in fact related to that commit.
>>>
>>>
>>>
>>>
>>>
>>> *From:* Anna Zaks [mailto:ganna at apple.com]
>>> *Sent:* 20 September 2014 01:19
>>> *To:* Artyom Skrobov
>>> *Cc:* cfe-commits at cs.uiuc.edu Commits; Ted Kremenek; Jordan Rose;
>>> Alexander Kornienko
>>> *Subject:* Re: [PATCH] Inverse post-order traversal for LiveVariables
>>> analysis, to recover the performance after r214064
>>>
>>>
>>>
>>> Hi Artyom,
>>>
>>>
>>>
>>> Unfortunately, this commit (r215650) causes major performance
>>> regressions on our buildbots. In particular, building postgresql-9.1 times
>>> out.
>>>
>>>
>>>
>>> Please, revert as soon as possible.
>>>
>>>
>>>
>>> Thank you,
>>>
>>> Anna.
>>>
>>> On Aug 20, 2014, at 3:13 AM, Alexander Kornienko <alexfh at google.com>
>>> wrote:
>>>
>>>
>>>
>>> On Fri, Aug 15, 2014 at 10:38 AM, Artyom Skrobov <Artyom.Skrobov at arm.com>
>>> wrote:
>>>
>>> Many thanks -- committed as r215650
>>>
>>> Alexander, can you confirm that the analyzer performance is now
>>> acceptable
>>> for your use cases?
>>>
>>>
>>>
>>> Artyom, sorry for the long delay. These files now work fine, but I still
>>> see up to 8-10 hours analysis time on a couple of other files. I'm sure I
>>> didn't see this before your first patch, but I can't yet tell in which
>>> revision it was introduced. I'll post more details and a repro later today.
>>>
>>>
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Ted kremenek [mailto:kremenek at apple.com]
>>> Sent: 14 August 2014 16:36
>>> To: Artyom Skrobov
>>> Cc: Alexander Kornienko; cfe-commits at cs.uiuc.edu
>>> Subject: Re: [PATCH] Inverse post-order traversal for LiveVariables
>>> analysis, to recover the performance after r214064
>>>
>>> Looks great to me.
>>>
>>> > On Aug 14, 2014, at 3:08 AM, Artyom Skrobov <Artyom.Skrobov at arm.com>
>>> wrote:
>>> >
>>> > Thank you Ted!
>>> >
>>> > Attaching the updated patch for a final review.
>>> >
>>> > Summary of changes:
>>> >
>>> > * Comments updated to reflect the two possible CFG traversal orders
>>> > * PostOrderCFGView::po_iterator taken out of the header file
>>> > * Iteration order for PostOrderCFGView changed to "reverse inverse
>>> > post-order", the one required for a backward analysis
>>> > * ReversePostOrderCFGView created, with the same iteration order that
>>> > PostOrderCFGView used to have, the one required for a forward analysis
>>> > * The two previous consumers of PostOrderCFGView, ThreadSafetyCommon.h
>>> and
>>> > Consumed.cpp, switched to use ReversePostOrderCFGView
>>> > * DataflowWorklistBase renamed to DataflowWorklist, and the two
>>> > specializations named BackwardDataflowWorklist and
>>> ForwardDataflowWorklist
>>> >
>>> > I believe this naming scheme matches the accepted terminology best.
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140921/13a17845/attachment.html>