Chromium debug build is very slow running Javascript |
||||
Issue descriptionChrome Version: 59.0.3047.0 (Developer Build) (64-bit) OS: (e.g. Win7, OSX 10.9.5, etc...) What steps will reproduce the problem? (1) Do a release and debug build of Chromium (2) Load third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/sample-accurate-scheduling.html (3) See how long it takes to run the test in both release and debug builds. (4) In the debug build add the command-ling flag --enable-features=V8NoTurbo and compare the runtimes again. What is the expected result? The debug build without and without Turbofan should be roughly the same, not 3 times longer with Turbofan. What happens instead? On a Z620, a debug build takes about 20-25 sec to run the test, With Turbofan disabled, it takes about 8 sec. This causes the test timeout when run locally. (Not sure why this isn't seen on the buildbots). It could also be the cause of MSAN bots reporting flaky tests recently, which never used to happen.
,
Mar 23 2017
,
Mar 23 2017
,
Mar 23 2017
This is due to the function window.Audit.prototype.beEqualToArray which internally does:
662 for (let index in this._actual) {
663 if (this._actual[index] !== this._expected[index])
664 errorIndices.push(index);
665 }
And that's called on typed arrays. For/In has to convert keys (index) to strings (typeof index == "string"). I had changed how expected[index] and actual[index] dealt with index being strings, which caused all those indicates (in string shape) to be internalized.
I fixed this, which improves the test from ~25 to ~4 seconds.
If you additionally change it to the following, it runs in ~1 second:
let actual = this._actual;
let expected = this._expected;
for (let index = 0; index < actual.length; index++) {
if (actual[index] != expected[index])
errorIndices.push(index);
}
,
Mar 23 2017
verwaest@ Can I ask where this is fixed?
,
Mar 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/20a803fd3c82fe6f644db4668f3b61f5e53b5958 commit 20a803fd3c82fe6f644db4668f3b61f5e53b5958 Author: Toon Verwaest <verwaest@chromium.org> Date: Thu Mar 23 16:51:38 2017 [runtime] Make sure we don't internalize string-encoded indices on KeyedGetProperty BUG= chromium:703226 Change-Id: I2232d4a721beb35478066b25143b9635bcc6b238 Reviewed-on: https://chromium-review.googlesource.com/458429 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#44073} [modify] https://crrev.com/20a803fd3c82fe6f644db4668f3b61f5e53b5958/src/runtime/runtime-object.cc
,
Mar 23 2017
c#4 sounds like a missed optimization opportunity for the compiler (converting the for-in-array to for index loop). (I don't know anything about compilers, so it might not really be possible or very hard to do.)
,
Mar 23 2017
Indeed, that optimization is pretty hard to do. And the alternative will always be faster in unoptimized code.
,
Mar 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/97300c5c38f775bdec197909e4140ce649c5713a commit 97300c5c38f775bdec197909e4140ce649c5713a Author: Toon Verwaest <verwaest@chromium.org> Date: Fri Mar 24 14:05:29 2017 [runtime] Immediately set the array-index hash in Uint32ToString and convert to string in ForInPrepare This speeds up for/in over arrays. E.g.,: function f(a) { for (let i in a) { if (a[i] != a[i]) print(false); } } var a = new Array(10000000); a.fill(1); f(a); runs 3x faster after the change. BUG= chromium:703226 Change-Id: Iabc5e931d985a03f89440cd702b2feb3eb9f5c18 Reviewed-on: https://chromium-review.googlesource.com/459538 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#44108} [modify] https://crrev.com/97300c5c38f775bdec197909e4140ce649c5713a/src/builtins/builtins-forin-gen.cc [modify] https://crrev.com/97300c5c38f775bdec197909e4140ce649c5713a/src/factory.h [modify] https://crrev.com/97300c5c38f775bdec197909e4140ce649c5713a/src/objects.cc [modify] https://crrev.com/97300c5c38f775bdec197909e4140ce649c5713a/src/runtime/runtime-forin.cc
,
Mar 24 2017
I tweaked the perf for for-in over array, and HasProperty for typed arrays. Combined this makes
function f(a) {
for (let i in a) {
if (a[i] != a[i]) print(false);
}
}
var a = new Uint32Array(10000000);
f(a);
run on my desktop in ~920ms down from >3s.
Using
function f(a) {
for (let i = 0; i < a.length; i++) {
if (a[i] != a[i]) print(false);
}
}
var a = new Uint32Array(10000000);
f(a);
is still much faster at ~40ms though :)
,
Mar 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/885f0bc4a77692e5aa3b2c0cad0c9f5b3ada3796 commit 885f0bc4a77692e5aa3b2c0cad0c9f5b3ada3796 Author: Toon Verwaest <verwaest@chromium.org> Date: Fri Mar 24 14:37:29 2017 [csa] Support typed arrays in Has(Own)Property BUG= chromium:703226 Change-Id: Ic71ae018c0090b2142f1d732bcf9c94c65e81f83 Reviewed-on: https://chromium-review.googlesource.com/458917 Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#44110} [modify] https://crrev.com/885f0bc4a77692e5aa3b2c0cad0c9f5b3ada3796/src/builtins/builtins-object-gen.cc [modify] https://crrev.com/885f0bc4a77692e5aa3b2c0cad0c9f5b3ada3796/src/code-stub-assembler.cc [modify] https://crrev.com/885f0bc4a77692e5aa3b2c0cad0c9f5b3ada3796/src/code-stub-assembler.h [modify] https://crrev.com/885f0bc4a77692e5aa3b2c0cad0c9f5b3ada3796/test/cctest/test-code-stub-assembler.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by rmcilroy@chromium.org
, Mar 20 2017Components: Blink>JavaScript>Compiler