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

[Artyom Skrobov]

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?

 

 

From: Anna Zaks [mailto:ganna at apple.com] 
Sent: 20 September 2014 18:58
To: Artyom Skrobov
Cc: 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> 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] 
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/20140922/0a306f1a/attachment.html>