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

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
HW: All
NextAction: ----
OS: All
Priority: 1
Type: Bug

Blocking:
issue 6936



Sign in to add a comment

Lots of %SetProperty calls in prepack test

Project Member Reported by bmeu...@chromium.org, Oct 24 2017

Issue description

In the prepack test of the web-tooling-benchmark we hit the %SetProperty runtime function quite often, and it seems that most of these hits are due to dictionary mode objects again.
 
wtb-prepack.js
6.5 MB View Download
prepack.json
11.2 MB View Download
wtb-prepack.png
171 KB View Download
Description: Show this description
Summary: Lots of %SetProperty calls in prepack test (was: Lot's of %SetProperty calls in prepack test)
Cc: bmeu...@chromium.org mvstan...@chromium.org
Owner: tebbi@chromium.org
Status: Assigned (was: Started)
tebbi@ agreed to take a look.

Comment 5 by tebbi@chromium.org, Nov 2 2017

The overhead is caused by non-internalized and transitioning stores in the setupBindings() function in prepack: 

this[propName + "_binding"] = { descriptor: desc, object: this, key: propName };


I attachaed a micro-benchmark that highlights the problem.
copy-object.js
197 bytes View Download
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 3 2017

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

commit 113527b6bc4e489e2ff23ec8d0b30cbabbf5ebd0
Author: Tobias Tebbi <tebbi@chromium.org>
Date: Fri Nov 03 09:25:45 2017

[ic] Internalize strings on the fly in KeyedStoreICGeneric

Internalizing a key in the KeyedStoreICGeneric avoids an expensive SetProperty runtime call. 
This improves the prepack benchmark by ~5%. 
In the micro-benchmark copy-object.js attached to the bug, it surfaces as a ~2.5x improvement.
The performance improvement currently relies on the stub cache, since we don't search for 
transitions from within the CSA. As this CL puts additional stress on the stub cache, 
performance regressions wouldn't be too surprising.

Bug: v8:6936,  v8:6997 
Change-Id: Id1469499a3ae5450519ff40d3c5a0915c6de0d45
Reviewed-on: https://chromium-review.googlesource.com/749951
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49097}
[modify] https://crrev.com/113527b6bc4e489e2ff23ec8d0b30cbabbf5ebd0/src/ic/keyed-store-generic.cc

Comment 7 by tebbi@chromium.org, Nov 17 2017

Status: Fixed (was: Assigned)

Sign in to add a comment