New issue
Advanced search Search tips

Issue 914395 link

Starred by 2 users

Issue metadata

Status: Duplicate
Owner:
Closed: Dec 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Array.prototype.splice slowdown 10x in v71

Reported by pa...@blacklabel.pl, Dec 12

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.80 Safari/537.36

Steps to reproduce the problem:
1. Open jsFiddle: http://jsfiddle.net/BlackLabel/2w7ujsp4/
2. Open js console
3. Click "Add Points"
4. Observe time results in console

What is the expected behavior?
Code execution time similar to v70.

What went wrong?
In v70, demo above takes ~130ms, in v71 it takes ~1400ms. 

Dealing with more points (300k):
- v70 -> ~25s
- v71 -> ~6-7minutes

Sometimes page crash can be observed (use even more points).

Did this work before? Yes 70.0.3538.0

Chrome version: 71.0.3578.80  Channel: stable
OS Version: OS X 10.13.6
Flash Version: 

Note: Demo is simplified and doesn't make sense but it's just a show case. If you prefer real use-case (with a lot of code) - let me know.
 
Broken by fd334b3216488011b368ec4652819e08c38d0d36 via r588623
Fixed by 696b2ceddd38afbff8776895cf9aadeea960d84f via r598616

For some weird reason the fix isn't present in Google Chrome 71, but present in Chromium 71.
Usually it's the other way around.

It's because V8 was reverted in the official Chrome to an older version 7.1.302 in r598739.
Components: -Blink Blink>JavaScript
Labels: Needs-Bisect Needs-Triage-M71
Cc: mvstan...@chromium.org phanindra.mandapaka@chromium.org
Labels: -Pri-2 -Needs-Bisect RegressedIn-71 ReleaseBlock-Stable Triaged-ET Target-71 M-71 FoundIn-71 hasbisect OS-Linux OS-Windows Pri-1
Owner: tebbi@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on reported chrome 71.0.3578.80 however issue seems to be fixed in latest chrome canary 73.0.3638.0 using Mac 10.14.0, Windows 10 and Ubuntu 17.10 with steps mentioned in comment# 0, hence providing reverse bisect info

Reverse Bisect Info:
================
Last Bad build:  71.0.3578.98
First Good build: 72.0.3579.0

Based on C#1, this is fixed in M-72 onwards by https://chromium-review.googlesource.com/c/v8/v8/+/1243104, hence assigning to tebbi@(reviewer of the CL) as mvstanton's@ last visit is showing more than 30 days ago.

Adding RB-Stable for M-71 for tracking and requesting for confirmation whether we should get the CL merged to M-71 if we plan anymore stable refresh.

Thanks..!
Cc: pbomm...@chromium.org hablich@chromium.org

+hablich@ (V8 TPM)
Labels: OS-Android OS-Chrome
Labels: Merge-Approved-71
FYI: No extra respin needed for this, but it should be part of a respin if it happens.
Mergedinto: 880780
Status: Duplicate (was: Assigned)
Merged to 71 

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

commit 366394e4299bbf0f786ec94b909e7e64a43663f2
Author: Mike Stanton <mvstanton@chromium.org>
Date: Thu Dec 13 17:07:13 2018

Merged: [Builtins] Array.prototype.splice performance improvements

Revision: 696b2ceddd38afbff8776895cf9aadeea960d84f

BUG= chromium:880780 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=tebbi@chromium.org

Change-Id: I9357601772c3214f5afa716c68d0f2a02b52a2a1
Reviewed-on: https://chromium-review.googlesource.com/c/1375917
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/branch-heads/7.1@{#65}
Cr-Branched-From: f70aaa8ab2e8815505a6145c745e50d8328cd28c-refs/heads/7.1.302@{#1}
Cr-Branched-From: 1dbcc78efa17a9047f7e923958087ef9eec43066-refs/heads/master@{#56462}
[modify] https://crrev.com/366394e4299bbf0f786ec94b909e7e64a43663f2/src/builtins/array-splice.tq
[modify] https://crrev.com/366394e4299bbf0f786ec94b909e7e64a43663f2/src/builtins/base.tq
[modify] https://crrev.com/366394e4299bbf0f786ec94b909e7e64a43663f2/src/code-stub-assembler.cc
[modify] https://crrev.com/366394e4299bbf0f786ec94b909e7e64a43663f2/src/code-stub-assembler.h

Labels: -Merge-Approved-71 Merge-Merged

Sign in to add a comment