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

[Anna Zaks]

On our builedbot that analyzes several projects (such as openssl, adium, postgresql), we see a 17% slowdown after r214064 and time out after r215650.

The commit message for r214064 states that it's "Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses". A simple factoring should not cause the 17% slowdown. 

Let's revert both patches while we are investigating what's going on.

Anna.
> On Sep 22, 2014, at 11:06 AM, Anna Zaks <ganna at apple.com> wrote:
> 
>> 
>> On Sep 22, 2014, at 10:03 AM, Artyom Skrobov <Artyom.Skrobov at arm.com <mailto:Artyom.Skrobov at arm.com>> wrote:
>> 
>> Alexander, I’m now looking at your example, thank you!
>>  
>> Anna, my question about the revision was to make sure I understand which patch you want reverted.
>> There were two patches, with a few weeks in between the two commits.
>> The first (r215650, Jul 28) introduced a performance regression, the second (r215650, Aug 14) fixed it for most but evidently not all cases.
>>  
>> When did PostgreSQL start timing out?
>>  
> 
> It started timing out after r215650. I'll look into the performance implications of 214064 on our side.
> 
> Anna.
>>  
>> From: Anna Zaks [mailto:ganna at apple.com <mailto:ganna at apple.com>] 
>> Sent: 20 September 2014 18:58
>> To: Artyom Skrobov
>> Cc: cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>; Alexander Kornienko
>> Subject: Re: [PATCH] Inverse post-order traversal for LiveVariables analysis, to recover the performance after r214064
>>  
>> Artyom,
>>  
>> PostgreSQL started timing out or taking a VERY long time. We have a Bulidbot that builds several projects and none of them were timing out before this commit. I don't know the specific revision; but it is PostgreSQL 9.1.
>>  
>> I suggest reverting this commit and investigating why it causes the regression. Generally, we should come up with a solution that does not take hours on any of the benchmarks.
>>  
>> Anna.
>> 
>> Sent from my iPhone
>> 
>> On Sep 20, 2014, at 8:10 AM, 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.
>>  
>>  
>> I suspect it is related.
>> 
>> 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>
> _______________________________________________
> 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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140922/eb83b3da/attachment.html>