New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: Yesterday
Cc:
Components:
HW: All
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 Back to list
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
tebbi@ agreed to take a look.
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
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, Yesterday (29 hours ago)
Status: Fixed
Sign in to add a comment