Commit 08db4d76 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[ic] Properly handle polymorphic symbol accesses.

Until now keyed accesses to properties with string or symbol keys were
only optimized properly while the IC was monomorphic and would go
megamorphic as soon as there's another receiver map, even if the name
was still the same (i.e. the same symbol or internalized string). This
was a weird performance-cliff, that'll hurt modern code especially
because for symbols you can only access them via keyed loads and stores.

This CL fixes the state machine inside the ICs to properly transition to
POLYMORPHIC state (and stay there) as long as the new name matches the
previously recorded name. The FeedbackVector and TurboFan were already
able to deal with this and didn't need any updates.

On the micro-benchmark from the tracking bug we go from

  testStringMonomorphic: 429 ms.
  testSymbolMonomorphic: 431 ms.
  testStringPolymorphic: 429 ms.
  testSymbolPolymorphic: 5621 ms.

to

  testStringMonomorphic: 429 ms.
  testSymbolMonomorphic: 429 ms.
  testStringPolymorphic: 429 ms.
  testSymbolPolymorphic: 430 ms.

effectively eliminating the overhead for symbols completely, and
yielding a 13.5x performance boost.

This also seems to yield a 1% improvement on the ARES6 ML benchmark,
because it eliminates the KEYED_LOAD_ICs for the Symbol.species lookups.

Bug: v8:6367, v8:6278, v8:6344
Change-Id: I879fe56387b4c56203c1ad8ef8cafb6cc4c32897
Reviewed-on: https://chromium-review.googlesource.com/695108Reviewed-by: 's avatarMichael Stanton <mvstanton@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48261}
parent d3c98121
......@@ -501,7 +501,9 @@ static bool AddOneReceiverMapIfMissing(MapHandles* receiver_maps,
bool IC::UpdatePolymorphicIC(Handle<Name> name, Handle<Object> handler) {
DCHECK(IsHandler(*handler));
if (is_keyed() && state() != RECOMPUTE_HANDLER) return false;
if (is_keyed() && state() != RECOMPUTE_HANDLER) {
if (nexus()->FindFirstName() != *name) return false;
}
Handle<Map> map = receiver_map();
MapHandles maps;
ObjectHandles handlers;
......@@ -539,10 +541,10 @@ bool IC::UpdatePolymorphicIC(Handle<Name> name, Handle<Object> handler) {
}
number_of_valid_maps++;
if (number_of_valid_maps > 1 && is_keyed()) return false;
if (number_of_valid_maps == 1) {
ConfigureVectorState(name, receiver_map(), handler);
} else {
if (is_keyed() && nexus()->FindFirstName() != *name) return false;
if (handler_to_overwrite >= 0) {
handlers[handler_to_overwrite] = handler;
if (!map.is_identical_to(maps.at(handler_to_overwrite))) {
......@@ -609,10 +611,8 @@ void IC::PatchCache(Handle<Name> name, Handle<Object> handler) {
}
// Fall through.
case POLYMORPHIC:
if (UpdatePolymorphicIC(name, handler)) break;
if (!is_keyed() || state() == RECOMPUTE_HANDLER) {
if (UpdatePolymorphicIC(name, handler)) break;
// For keyed stubs, we can't know whether old handlers were for the
// same key.
CopyICToMegamorphicCache(name);
}
ConfigureVectorState(MEGAMORPHIC, name);
......
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax
(function() {
const symbol = Symbol('symbol');
const OBJS = [
{[symbol]: 0, a: 1},
{[symbol]: 1, b: 2},
{[symbol]: 2, c: 3},
{[symbol]: 3, d: 4}
];
function foo(o) { return o[symbol]; }
for (let i = 0; i < OBJS.length; ++i) {
assertEquals(i, foo(OBJS[i]));
assertEquals(i, foo(OBJS[i]));
}
%OptimizeFunctionOnNextCall(foo);
for (let i = 0; i < OBJS.length; ++i) {
assertEquals(i, foo(OBJS[i]));
assertEquals(i, foo(OBJS[i]));
}
})();
(function() {
const symbol = Symbol('symbol');
const OBJS = [
{[symbol]: 0, a: 1},
{[symbol]: 1, b: 2},
{[symbol]: 2, c: 3},
{[symbol]: 3, d: 4}
];
function foo(o) { o[symbol] = o; }
for (let i = 0; i < OBJS.length; ++i) {
foo(OBJS[i]);
foo(OBJS[i]);
}
%OptimizeFunctionOnNextCall(foo);
for (let i = 0; i < OBJS.length; ++i) {
foo(OBJS[i]);
foo(OBJS[i]);
}
for (const o of OBJS) {
assertEquals(o, o[symbol]);
}
})();
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