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

[Anna Zaks]

The analyzer is still timing out for me on postgresql.

I am going to send a preprocessed file off line.

Maybe you will find some answers by looking at other performance related patches for LiveVariables that have been committed over years. 

Anna.
> On Oct 2, 2014, at 4:45 AM, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:
> 
> Hello,
>  
> I see now that the CFG iteration orders, so painstakingly defined during the previous round of reviews in Aug, didn’t entirely match the pre-r214064 behaviour.
>  
> Attached is a patch that simplifies PostOrderCFGView significantly, making the two possible iteration orders really obvious.
>  
> I have tested it with both Alexander’s testcases (analyzer-regression1.c from Sep, and options.c from Aug), and it shows good performance on both.
>  
> OK to un-revert r218296, and to commit this patch on top of that?
>  
>  
> From: Alexander Kornienko [mailto:alexfh at google.com] 
> Sent: 21 September 2014 01:16
> To: Artyom Skrobov
> Cc: Anna Zaks; cfe-commits at cs.uiuc.edu Commits
> Subject: Re: [PATCH] Inverse post-order traversal for LiveVariables analysis, to recover the performance after r214064
>  
> 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 <mailto: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 <mailto: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 <mailto:ganna at apple.com>] 
> Sent: 20 September 2014 01:19
> To: Artyom Skrobov
> Cc: cfe-commits at cs.uiuc.edu <mailto: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 <mailto:alexfh at google.com>> wrote:
>  
> On Fri, Aug 15, 2014 at 10:38 AM, Artyom Skrobov <Artyom.Skrobov at arm.com <mailto: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 <mailto:kremenek at apple.com>]
> Sent: 14 August 2014 16:36
> To: Artyom Skrobov
> Cc: Alexander Kornienko; cfe-commits at cs.uiuc.edu <mailto: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 <mailto: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 <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>  
> <PostOrderCFGView.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141003/7a2e33f0/attachment.html>