Project: v8 Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 3 users
Status: Fixed
Owner:
Closed: Aug 10
Cc:
Components:
HW: All
OS: All
Priority: 1
Type: Bug
ES5



Sign in to add a comment
Freezing the Object.prototype or Array.prototype triggers slow path for several Array builtins
Project Member Reported by bmeu...@chromium.org, Aug 9 Back to list
Invoking Object.freeze on either the Object.prototype or the Array.prototype changes its elements backing store to DICTIONARY_ELEMENTS kind, which is not properly checked in all places where we test for elements in the prototype chain, i.e. in JSObject::PrototypeHasNoElements.

This causes several Array builtins to take the slow path, i.e. Array.prototype.splice:

==========================================
function test(a) {
  for (var i = 0; i < 1e6; ++i) {
    a = a.splice();
  }
  return a;
}

const n = 1e6;
const a = new Array(n);
for (var i = 0; i < n; ++i) a[i] = i;

console.time('before');
test(a);
console.timeEnd('before');

Object.freeze(Object.prototype);

console.time('after');
test(a);
console.timeEnd('after');
==========================================

Running this locally you can immediately see the impact:

==========================================
$ out/Release/d8 splice.js
before: 86.111000
after: 375.197000
==========================================

Even worse with Array.prototype.slice:

==========================================
function test(a) {
  for (var i = 0; i < 1e6; ++i) {
    a = a.slice();
  }
  return a;
}

const n = 1e2;
const a = new Array(n);
for (var i = 0; i < n; ++i) a[i] = i;

console.time('before');
test(a);
console.timeEnd('before');

Object.freeze(Object.prototype);

console.time('after');
test(a);
console.timeEnd('after');
==========================================

Here we see a drastic performance hit:

==========================================
$ out/Release/d8 slice.js
before: 197.807000
after: 8280.652000
==========================================

 
This is a great find!
Just for completeness: CodeStubAssembler::BranchIfPrototypesHaveNoElements is also affected.
Project Member Comment 2 by bugdroid1@chromium.org, Aug 10
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/293283d55983b421fe6e246ae22d4de531f429ec

commit 293283d55983b421fe6e246ae22d4de531f429ec
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Thu Aug 10 03:49:12 2017

[builtins] Fix no elements check on the prototype chain.

Invoking Object.freeze on either the Object.prototype or the
Array.prototype changes its elements backing store to
DICTIONARY_ELEMENTS kind, which is not properly checked in all
placeswhere we test for elements in the prototype chain, i.e. in
JSObject::PrototypeHasNoElements. This causes several Array
builtins to take the slow path, i.e. Array.prototype.splice.

Fix this for now by consistently checking for either empty_fixed_array
or empty_slow_element_dictionary in both C++ and CSA runtime.

Bug:  v8:6689 
Change-Id: I3f62643131b3a874b5c2a3d7ed054dd1e799bbaf
Reviewed-on: https://chromium-review.googlesource.com/608127
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47264}
[modify] https://crrev.com/293283d55983b421fe6e246ae22d4de531f429ec/src/code-stub-assembler.cc
[modify] https://crrev.com/293283d55983b421fe6e246ae22d4de531f429ec/src/objects-inl.h

Status: Fixed
Sign in to add a comment