New issue
Advanced search Search tips

Issue 816522 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

4.8% regression in octane at 538977:539003

Project Member Reported by tdres...@chromium.org, Feb 26 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Feb 26 2018

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

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


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

chromium-rel-win7-dual
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Feb 27 2018

Cc: bmeu...@chromium.org jgruber@chromium.org sigurds@chromium.org gsat...@chromium.org
Owner: jgruber@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/148a8f70440000

[turbofan] Introduce StringSubstring operator by sigurds@chromium.org
https://chromium.googlesource.com/v8/v8/+/bcb5d452102d268a7976a9f4da73864fc0369ff8

Remove unused runtime functions by jgruber@chromium.org
https://chromium.googlesource.com/v8/v8/+/1ee80ebab0ca075d56061311b2056e5dc85c56eb

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Cc: -sigurds@chromium.org
Owner: sigurds@chromium.org
Status: WontFix (was: Assigned)
Might be related to change in computing indices as word32 instead of Smi. Only affected platform is i32.
Can you provide details on why this was WontFixed?
Status: Started (was: WontFix)
More investigation shows this is related to GVN hoisting a call to String.p.substring out of a switch-case. Fix in flight.
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 27 2018

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

commit 20baf1bfb7a9faf987bbdf857d6764ce33c58dcc
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Tue Feb 27 17:45:24 2018

[turbofan] Put StringSubstring on the effect chain

This change ensures that GVN does not move StringSubstring out of
switches, which might introduce partial redundancies.

Bug:  chromium:816522 
Change-Id: I63b91edd995c84b68d756ed5de08fa13567f3d80
Reviewed-on: https://chromium-review.googlesource.com/939621
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51610}
[modify] https://crrev.com/20baf1bfb7a9faf987bbdf857d6764ce33c58dcc/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/20baf1bfb7a9faf987bbdf857d6764ce33c58dcc/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/20baf1bfb7a9faf987bbdf857d6764ce33c58dcc/src/compiler/simplified-operator.cc

Status: Fixed (was: Started)
If I interpret the graph correctly, this had been fixed. Can you verify?
Yup, looks recovered to me.

Sign in to add a comment