Is there a way to test the benchmark in Chrome on Android? Is there an HTML file or website containing the test case?

Yes you can select Chrome as the browser to use in the app.  I'm doing a per cl bisect now.

Caused by v8 roll (as found out in the linked bug): https://codereview.chromium.org/2335343003

I'll take a look at this for now since I'm the WebView bug cop this week.

Alright, it looks like this is caused by
https://codereview.chromium.org/2330063002
submitted by ishell@, so assigning to them.

I ran the benchmarks with a Nexus 6, using WebView, on NRD88. Running Vellamo 3.2.6.

The total I get is 2612 @ commit d7ee8124e8d1d9d742039d59a001d055eaaf8066
and
2361 @ commit cce56a3f470d4f8f4c241208b166dea5e09d0620

The main difference seems to be the benchmark "Crossfader" which goes down from 223 to 13 (higher scores are better).

ishell@ could you take a look at this? This bug was originally filed in b/32643460 since Android test teams run Vellamo benchmarks for WebView.
According to that bug this should be device- and OS non-specific.


The results for the tiny bisect I did are here:
https://docs.google.com/a/google.com/document/d/1pSv6HiJ-GJ81dyW_RiOjBpP54FlJ9Rs6-HSl1QWOXY8/edit?usp=sharing

Bisected breaking V8 commit to:

commit cce56a3f470d4f8f4c241208b166dea5e09d0620
Author: ishell <ishell@chromium.org>
Date:   Wed Sep 14 02:28:04 2016 -0700

    [stubs] Port StoreFastElementsStub to TurboFan.
    
    This CL adds CSA::Retain() operation that ensures that the value is kept alive even during GC.
    
    BUG=v8:5269
    
    Review-Url: https://codereview.chromium.org/2330063002
    Cr-Commit-Position: refs/heads/master@{#39407}

Wild guess: In a ported code stores to clamped uint8 arrays are always generated as a sequence of two checks while the old code used usat instruction on Arm. 

I'll try to reproduce it locally and see what I can do.

Interesting. That'd also affect the inlined code then (i.e. the TurboFan version of Uint8ClampedArray stores). We could add an optional machine operator for the usat conversion, that is provided on ARM.

We still have some runway to take M55 merges before stable, so if this regression is genuine and will impact real-world performance, we should discuss taking a merge.

Ok, I see the regression. Investigating...

Is there anything that needs to be restricted in this bug? The Android team can't see it currently which makes it hard for them to track. Can we unrestrict it if not?

I think we can make it public but I don't know how the Clank team normally handles this. amineer@ any insight?

Nothing private that I can see, lifting R-V-G.

Why is this still pending?  We need a fix yesterday if this is going to block M55.  PTAL ASAP.

I've been working on this for the last few days and tried a couple of ideas which didn't work out. But today I think I finally figured out what caused the regression I just have to understand why. I think I'll be able to figure that out tomorrow.

The reason of a slowdown is that the new stub bails out when we try to store a double value to an int typed array which in turn makes the KeyedStoreIC to generic (since we get to the miss handler with the same receiver map).

We're planning to cut M55 pre-Stable RC for Desktop today. will this be a blocker for Desktop pre-stable release this week?

I spoke with ishell@ this AM and they noted this: "We didn't see this regression on any of our benchmarks, so probably it's not that bad."

That said this is a Qualcomm benchmarking suite, so I'd like to be sure we have no impact before shipping to millions of users.  benhenry@ will facilitate getting a call made there by EOD.

In the meantime ishell@ will work to have a safe fix with a goal of landing on trunk by Thursday, to be able to make respins (as needed) next week.

Speed Releasing believes that there is no convincing user impact with this regression. We'll continue to investigate and see if there is something we missed given the short amount of time we've had to investigate. That said, there's nothing major that would force us to stop shipping the release at this point.

Thank you benhenry@. We will move forward with M55 Desktop pre-stable release as plan. 

+1, thanks benhenry@.  ishell@, this means we should only take a change for 55 if it's very safe; if it comes with any risk, please let us know so we can discuss options.

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

commit c819616376b6835b61ff4f9e2667ba94ab25fe7f
Author: ishell <ishell@chromium.org>
Date: Wed Nov 30 15:23:49 2016

[ic] Prevent KeyedStoreIC from being generic when storing doubles to integer typed arrays.

BUG= chromium:666947 

Review-Url: https://codereview.chromium.org/2539013002
Cr-Commit-Position: refs/heads/master@{#41390}

[modify] https://crrev.com/c819616376b6835b61ff4f9e2667ba94ab25fe7f/src/code-stub-assembler.cc
[modify] https://crrev.com/c819616376b6835b61ff4f9e2667ba94ab25fe7f/src/code-stub-assembler.h
[modify] https://crrev.com/c819616376b6835b61ff4f9e2667ba94ab25fe7f/src/compiler/code-assembler.cc
[modify] https://crrev.com/c819616376b6835b61ff4f9e2667ba94ab25fe7f/src/compiler/code-assembler.h

(Old  issue 331620  mentions similar problem with the crossfade benchmark)

That patch is huge to be taking right before stable (for M55); if it were a Chromium patch I'd reject it outright.  Is that considered low-risk for V8?

It definitely requires test coverage. I got an unexpected "surprise" when I tried to check it on my Android. 
At least I think it's fine for M56.

According to #25 let's not merge it to 55.

M56 Beta promotion is scheduled on Dec 6 & RC cut on Monday, Dec 5 @ 4.00 PM PST.Please ensure to verify the fix and merge your change ASAP so that we could take it for next Release.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit b21d9e05344bc10f481a37fe3a0d2dccc2f04498
Author: ishell@chromium.org <ishell@chromium.org>
Date: Mon Dec 05 14:27:00 2016

Merged: [ic] Prevent KeyedStoreIC from being generic when storing doubles to integer typed arrays.

Revision: c819616376b6835b61ff4f9e2667ba94ab25fe7f

BUG= chromium:666947 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=cbruni@chromium.org

Review URL: https://codereview.chromium.org/2551903002 .

Cr-Commit-Position: refs/branch-heads/5.6@{#34}
Cr-Branched-From: bdd3886218dfe76e8560eb8a18401942452ae859-refs/heads/5.6.326@{#1}
Cr-Branched-From: 879f6599eee6e1dfcbe9a24bf688b261c03e9558-refs/heads/master@{#41014}

[modify] https://crrev.com/b21d9e05344bc10f481a37fe3a0d2dccc2f04498/src/code-stub-assembler.cc
[modify] https://crrev.com/b21d9e05344bc10f481a37fe3a0d2dccc2f04498/src/code-stub-assembler.h
[modify] https://crrev.com/b21d9e05344bc10f481a37fe3a0d2dccc2f04498/src/compiler/code-assembler.cc
[modify] https://crrev.com/b21d9e05344bc10f481a37fe3a0d2dccc2f04498/src/compiler/code-assembler.h

Issue 665392 has been merged into this issue.

If there is no pending work please remove Merge-Approved-56 label.

Verified on Samsung Note 5 MMB29K using WebView version 56.0.2924.18

After 55 is released we get user reports (https://github.com/mattgodbolt/jsbeeb/issues/138) that this is affecting existing code quite severe. Please merge to 5.5.

 Issue 673137  has been merged into this issue.

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

commit fc0a0d7699c039df53961d47291981b151aba44f
Author: ishell@chromium.org <ishell@chromium.org>
Date: Mon Dec 12 08:43:20 2016

Merged: [ic] Prevent KeyedStoreIC from being generic when storing doubles to integer typed arrays.

Revision: c819616376b6835b61ff4f9e2667ba94ab25fe7f

BUG= chromium:666947 ,  chromium:673137 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=bmeurer@chromium.org

Review-Url: https://codereview.chromium.org/2564323003 .
Cr-Commit-Position: refs/branch-heads/5.5@{#66}
Cr-Branched-From: 3cbd5838bd8376103daa45d69dade929ee4e0092-refs/heads/5.5.372@{#1}
Cr-Branched-From: b3c8b0ce2c9af0528837d8309625118d4096553b-refs/heads/master@{#40015}

[modify] https://crrev.com/fc0a0d7699c039df53961d47291981b151aba44f/src/code-stub-assembler.cc
[modify] https://crrev.com/fc0a0d7699c039df53961d47291981b151aba44f/src/code-stub-assembler.h
[modify] https://crrev.com/fc0a0d7699c039df53961d47291981b151aba44f/src/compiler/code-assembler.cc
[modify] https://crrev.com/fc0a0d7699c039df53961d47291981b151aba44f/src/compiler/code-assembler.h

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

It's already merged to M55.

 Issue 676005  has been merged into this issue.

 Issue 678936  has been merged into this issue.

No further action needed for Node.js. This issue doesn't affect V8 5.4 or older.

Is there a way to know when this has been rolled out? Apologies for the naive question, but as of 55.0.2883.87 I am still seeing the slowdown I reported (th jsbeeb issue above). It's not clear to me what merged-to-M55 means, and whether that means it ought to already be in v55. Thanks!

I don't think this has been rolled out yet. It will be on the next 55 or first 56 push.

My opinion is that this fix is urgent for 55. Many heavy processing webapps
are suffering a lot with this in Chrome.
This is not a minor inconvenience that can wait that much...

Apologies, we don't have a great way for you to look up what versions of Chrome a V8 change is included in aside from visiting omahaproxy.appspot.com, looking at the V8 tag of each shipping version, and checking the CLs that are associated with that tag in V8's source repo.  We're working on building some better tools to facilitate this lookup though.

That aside, the fix has been deployed to Chrome OS and Mac, but not Windows Android or Linux - not because we don't want to ship the fix, but because shipping updates is expensive for users, and so we try to only do so when there is a critical issue (which there was for Chrome OS and Mac, but not Windows or Android).  Aside from the bug report from the jsbeeb issue (which doesn't get into real world impact from a brief skim) we haven't seen many user reports of this problem, so we did not anticipate it being critical.  Can you please describe the types of performance issues you're experiencing on your production pages?

FWIW this issue is fixed in M56 and we're planning on rolling that out soon (https://www.chromium.org/developers/calendar) - Jan 31, with some platforms starting their initial deployments up to a week in advance - so you will not have to live with this very much longer at all even if we wait for M56.

Oh, if v56 is that close, I say we can wait.
I hope the other problems regarding the Full Screen Mode and the Standalone
Mode in Android that I reported in v56 beta get solved in time for the
release.

What bug #'s, ppeccin@?

 Issue 676634 .
I think there are 2 or 3 problems being discussed in this issue, all related to the Android System Bar not hiding correctly in FullScreen and Standalone modes.

There is also a problem related the URL Bar hiding and subsequent wrong page height when you go from Landscape to Portrait, but I can't find an issue for this specific problem.

If 56 is coming soon then great. From my perspective it's slightly unfortunate that I have to recommend people using non-Chrome browsers to access my site (it's unusuably slow with this regression), but for the 10 people a day that visit it, it's not worth inconveniencing everyone else!

Thanks everyone for your helpful replies.