All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=814182

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=7edc60e156dab66836dc0a5a21b0aff1bb4cab87594d9f52e403e69ca9731ddb


Bot(s) for this bug's original alert(s):

android-webview-nexus5X

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/11b98af7840000

📍 Found a significant difference after 1 commit.
https://chromeperf.appspot.com/job/11b98af7840000

Revert "[parser] Remove pretenuring of closures assigned to properties" by adamk@chromium.org
https://chromium.googlesource.com/v8/v8/+/53338cbef5e80302ad5ec3089c20c5de35124e10

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

This regression is due to a revert of a recent progression. It was reverted due to a performance regression in a Promise-heavy benchmark. See  issue v8:7442  for details.

This memory win suggests to me that we might want to have the patch even with a regression in the Promise benchmark (if that's indeed the only thing that regressed). +rmcilroy for his thoughts.

+bmeurer who's looking at the promise benchmarks

My strawperson suggestion, which I'd like to hear bmeurer and rmcilroy's thoughts on, would be to reland the removal of pretenuring (revert the revert, that is), and open a v8 bug (possibly owned by bmeurer, who's working on lots of Promise-related stuff) to find another way to fix bluebird-doxbee's reliance on this pretenuring heuristic. WDYT?

It kinda goes against our current OKR to improve on the promise benchmarks, but I'd be strongly in favor of removing the pretenuring flag now, since the improvement it brings is not really due to it's design, but just because we're lucky to save some scavenge cycle in the beginning of the benchmark (I think). Maybe we should just give the benchmark more time to warmup, so that these unrelated effects disappear.

Yup, I'm also in favour of removing the pretenuring of these closure objects and live with the benchmark regressions (which I agree with Benedkit are probably more benchmark related than actually impacting real-world promise performance) until we have a more general approach that doesn't have the memory impact.

Sounds like a plan. I'll reland the change after the branch point, just so it has more time to bake in case this long-standing heuristic has other negative consequences to be flushed out.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/3d7ad2e7e53d3cd6add51512580c054abf7e5805

commit 3d7ad2e7e53d3cd6add51512580c054abf7e5805
Author: Adam Klein <adamk@chromium.org>
Date: Thu Mar 01 21:24:57 2018

Reland "[parser] Remove pretenuring of closures assigned to properties"

The memory gains were significant, so despite the bluebird-doxbee
regression, we think it's better to have this patch than not.
See the attached Chromium bug for more discussion.

This is a reland of 20e346bd08591caff2f7c45de762ca4d4e3f72c8.

Original change's description:
> [parser] Remove pretenuring of closures assigned to properties
>
> This pretenuring was added in https://codereview.chromium.org/5220007,
> back when it was necessary in order to allow use of the closure
> as a "constant function" property. This should no longer be the case,
> and the pretenuring causes some unfortunate downstream effects.
>
> This patch removes the parser's setting of this bit. If it doesn't
> cause regressions on the perf bots, followup CLs will remove the
> rest of the support for this feature.
>
> Bug:  v8:7442 
> Change-Id: I27c43dd4293ce5de921be6c78571e712778d138a
> Reviewed-on: https://chromium-review.googlesource.com/914610
> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> Commit-Queue: Adam Klein <adamk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51254}

Bug:  v8:7442 ,  chromium:814182 
Change-Id: I228c59dccef3844803f115749e72ae6c5f286eda
Reviewed-on: https://chromium-review.googlesource.com/938241
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51668}
[modify] https://crrev.com/3d7ad2e7e53d3cd6add51512580c054abf7e5805/src/parsing/parser-base.h
[modify] https://crrev.com/3d7ad2e7e53d3cd6add51512580c054abf7e5805/src/parsing/parser.h
[modify] https://crrev.com/3d7ad2e7e53d3cd6add51512580c054abf7e5805/src/parsing/preparser.h

 Issue v8:7524  tracks the bluebird-doxbee regression.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5cf2ae5a4d7224198a0474eb27cc618b6843bc32

commit 5cf2ae5a4d7224198a0474eb27cc618b6843bc32
Author: Adam Klein <adamk@chromium.org>
Date: Mon Mar 26 19:09:22 2018

Revert "Reland "[parser] Remove pretenuring of closures assigned to properties""

This reverts commit 3d7ad2e7e53d3cd6add51512580c054abf7e5805.

Reason for revert: too many regressions to handle for now.

Original change's description:
> Reland "[parser] Remove pretenuring of closures assigned to properties"
>
> The memory gains were significant, so despite the bluebird-doxbee
> regression, we think it's better to have this patch than not.
> See the attached Chromium bug for more discussion.
>
> This is a reland of 20e346bd08591caff2f7c45de762ca4d4e3f72c8.
>
> Original change's description:
> > [parser] Remove pretenuring of closures assigned to properties
> >
> > This pretenuring was added in https://codereview.chromium.org/5220007,
> > back when it was necessary in order to allow use of the closure
> > as a "constant function" property. This should no longer be the case,
> > and the pretenuring causes some unfortunate downstream effects.
> >
> > This patch removes the parser's setting of this bit. If it doesn't
> > cause regressions on the perf bots, followup CLs will remove the
> > rest of the support for this feature.
> >
> > Bug:  v8:7442 
> > Change-Id: I27c43dd4293ce5de921be6c78571e712778d138a
> > Reviewed-on: https://chromium-review.googlesource.com/914610
> > Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> > Commit-Queue: Adam Klein <adamk@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#51254}
>
> Bug:  v8:7442 ,  chromium:814182 
> Change-Id: I228c59dccef3844803f115749e72ae6c5f286eda
> Reviewed-on: https://chromium-review.googlesource.com/938241
> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> Commit-Queue: Adam Klein <adamk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51668}

Tbr: gsathya@chromium.org
Bug:  v8:7442 ,  v8:7524 ,  chromium:814182 ,  chromium:818627 ,  chromium:818672 ,  chromium:819994 ,  chromium:821788 
Change-Id: Ib760d63f879613f3b874889c5cb29ba2a77ba430
Reviewed-on: https://chromium-review.googlesource.com/980795
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52233}
[modify] https://crrev.com/5cf2ae5a4d7224198a0474eb27cc618b6843bc32/src/parsing/parser-base.h
[modify] https://crrev.com/5cf2ae5a4d7224198a0474eb27cc618b6843bc32/src/parsing/parser.h
[modify] https://crrev.com/5cf2ae5a4d7224198a0474eb27cc618b6843bc32/src/parsing/preparser.h