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 <[email protected]> wrote: > >> >> On Sep 22, 2014, at 10:03 AM, Artyom Skrobov <[email protected] >> <mailto:[email protected]>> 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:[email protected] <mailto:[email protected]>] >> Sent: 20 September 2014 18:58 >> To: Artyom Skrobov >> Cc: [email protected] <mailto:[email protected]>; 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 <[email protected] >> <mailto:[email protected]>> 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:[email protected] <mailto:[email protected]>] >> Sent: 20 September 2014 01:19 >> To: Artyom Skrobov >> Cc: [email protected] <mailto:[email protected]> 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 <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Fri, Aug 15, 2014 at 10:38 AM, Artyom Skrobov <[email protected] >> <mailto:[email protected]>> 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:[email protected] <mailto:[email protected]>] >> Sent: 14 August 2014 16:36 >> To: Artyom Skrobov >> Cc: Alexander Kornienko; [email protected] >> <mailto:[email protected]> >> 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 <[email protected] >> > <mailto:[email protected]>> >> 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 >> [email protected] <mailto:[email protected]> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits> > _______________________________________________ > cfe-commits mailing list > [email protected] <mailto:[email protected]> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
