Commit e1163c14 authored by Mike Stanton's avatar Mike Stanton Committed by Commit Bot

[Builtins] Array.prototype.forEach perf regression on dictionaries

An unnecessary call to ToString() on the array index caused trips to
the runtime. The fix also includes performance micro-benchmarks so
we'll have a harder time regressing this case in future.

TBR=tebbi@chromium.org

Bug: v8:8112
Change-Id: I781e8b1bbe2eb56db961cf33b0dca8523868b83d
Reviewed-on: https://chromium-review.googlesource.com/1213207
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: 's avatarMichael Stanton <mvstanton@chromium.org>
Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55733}
parent b4b2dafc
......@@ -5,20 +5,21 @@
module array {
macro ArrayForEachTorqueContinuation(
context: Context, o: JSReceiver, len: Number, callbackfn: Callable,
thisArg: Object, initial_k: Smi): Object {
thisArg: Object, initial_k: Number): Object {
// 5. Let k be 0.
// 6. Repeat, while k < len
for (let k: Smi = initial_k; k < len; k = k + 1) {
for (let k: Number = initial_k; k < len; k = k + 1) {
// 6a. Let Pk be ! ToString(k).
const pK: String = ToString_Inline(context, k);
// k is guaranteed to be a positive integer, hence ToString is
// side-effect free and HasProperty/GetProperty do the conversion inline.
// 6b. Let kPresent be ? HasProperty(O, Pk).
const kPresent: Boolean = HasProperty(context, o, pK);
const kPresent: Boolean = HasProperty_Inline(context, o, k);
// 6c. If kPresent is true, then
if (kPresent == True) {
// 6c. i. Let kValue be ? Get(O, Pk).
const kValue: Object = GetProperty(context, o, pK);
const kValue: Object = GetProperty(context, o, k);
// 6c. ii. Perform ? Call(callbackfn, T, <kValue, k, O>).
Call(context, callbackfn, thisArg, kValue, k, o);
......@@ -60,7 +61,7 @@ module array {
try {
const callbackfn: Callable =
cast<Callable>(callback) otherwise Unexpected;
const k: Smi = cast<Smi>(initialK) otherwise Unexpected;
const k: Number = cast<Number>(initialK) otherwise Unexpected;
const number_length: Number = cast<Number>(length) otherwise Unexpected;
return ArrayForEachTorqueContinuation(
......@@ -159,7 +160,7 @@ module array {
const thisArg: Object = arguments.length > 1 ? arguments[1] : Undefined;
// Special cases.
let k: Smi = 0;
let k: Number = 0;
try {
return FastArrayForEach(context, o, len, callbackfn, thisArg)
otherwise Bailout;
......
......@@ -191,6 +191,7 @@ extern macro GetProperty(Context, Object, Object): Object;
extern builtin SetProperty(Context, Object, Object, Object);
extern builtin DeleteProperty(Context, Object, Object, LanguageMode);
extern builtin HasProperty(Context, JSReceiver, Object): Boolean;
extern macro HasProperty_Inline(Context, JSReceiver, Object): Boolean;
extern macro ThrowRangeError(Context, constexpr MessageTemplate): never;
extern macro ThrowTypeError(Context, constexpr MessageTemplate): never;
......
......@@ -2739,6 +2739,14 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
SloppyTNode<Object> key,
HasPropertyLookupMode mode);
// Due to naming conflict with the builtin function namespace.
TNode<Oddball> HasProperty_Inline(TNode<Context> context,
TNode<JSReceiver> object,
TNode<Object> key) {
return HasProperty(context, object, key,
HasPropertyLookupMode::kHasProperty);
}
Node* Typeof(Node* value);
TNode<Object> GetSuperConstructor(SloppyTNode<Context> context,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment