New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 902230 link

Starred by 2 users

Issue metadata

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

Blocking:
issue v8:6666



Sign in to add a comment

gc-incremental-finalize regression due to dispatch table iteration

Project Member Reported by jgruber@chromium.org, Nov 6

Issue description

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

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


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

linux-perf

v8.browsing_desktop - Benchmark documentation link:
  None
Blocking: 6666
I suspect this is due to re-enabling dispatch table iteration in 

https://chromium-review.googlesource.com/c/v8/v8/+/1304539/8/src/interpreter/interpreter.cc

How big/important is 20% in gc-incremental-finalize?
Blocking: -6666 v8:6666
Cc: jgruber@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1151f971e40000

[snapshot] Remove the builtins snapshot by jgruber@chromium.org
https://chromium.googlesource.com/v8/v8/+/4ef0e79cba592ee16d279587dc8393bfb229f995
v8-gc-incremental-finalize: 0.1351 → 0.1674 (+0.03228)

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

Benchmark documentation link:
  None
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 6

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

commit e89690554714e2ddbf240f3bffe828899fc854e8
Author: Dan Elphick <delphick@chromium.org>
Date: Tue Nov 06 12:55:35 2018

[snapshot] Clean up Strong root iteration in deserialization

When deserializing the startup snapshot, call IterateStrongRoots with
VISIT_FOR_SERIALIZATION rather than VISIT_ONLY_STRONG. To compensate,
make the StartupDeserializer explicitly iterate over the partial
snapshot cache.

This makes the deserializer and serializer consistent in their use of
the function and makes their differences explicit in the snapshot code
itself.

Bug:  chromium:902230 
Change-Id: I3a2ac858f4f6b3097b98a10ed2dd5ac5b9bf83e8
Reviewed-on: https://chromium-review.googlesource.com/c/1319585
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57275}
[modify] https://crrev.com/e89690554714e2ddbf240f3bffe828899fc854e8/src/heap/heap.cc
[modify] https://crrev.com/e89690554714e2ddbf240f3bffe828899fc854e8/src/snapshot/startup-deserializer.cc

Owner: delph...@chromium.org
Chatted with Ulan, the absolute regression (~0.04 ms) is too small to care (much). Dan, assigning to you since you were looking into disabling iteration. Feel free to close or fix as you like.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7

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

commit 47764c761fb5da99e58f2d5a56ab9dba8871efb7
Author: Dan Elphick <delphick@chromium.org>
Date: Wed Nov 07 10:44:43 2018

[heap] Skip offheap bytecode handlers for GC iteration

If builtins are embedded and we're not generating the snapshot, then
completely skip iterating over the dispatch table, since off-heap
bytecode handlers can never move or be collected.

Additionally the dispatch table is initialized elsewhere so skip
iterating over the table completely when serializing/deserializing.

Bug:  chromium:902230 
Change-Id: I2cfe5b4b325d100145d5759ff97e0c8dde7ed7a3
Reviewed-on: https://chromium-review.googlesource.com/c/1319750
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57312}
[modify] https://crrev.com/47764c761fb5da99e58f2d5a56ab9dba8871efb7/src/heap/heap.cc
[modify] https://crrev.com/47764c761fb5da99e58f2d5a56ab9dba8871efb7/src/interpreter/interpreter.cc

Status: Fixed (was: Assigned)
Cc: alexclarke@chromium.org
 Issue 903164  has been merged into this issue.

Sign in to add a comment