Adding/Modifying a method on javascript class prototype is slow on Chrome compared to Edge/Firefox
Reported by
mihir....@gmail.com,
Apr 26 2016
|
||||||
Issue description
Chrome Version : Version 49.0.2623.112 m
Other browsers tested:
Add OK or FAIL, along with the version, after other browsers where you
have tested this issue:
Safari:
Firefox: OK
IE: OK
Edge: OK
What steps will reproduce the problem?
Check the attached protopatch.html which has minimal code to represent what the msajax Type.resolveInheritance method does internally. (https://msdn.microsoft.com/en-us/library/bb310949(v=vs.100).aspx)
In this test we copy the one class prototype methods to the another class prototype (i.e msajax inheritance does similar thing).
The test page copies the base class methods to 1000 derived class prototype.
Open the page in all the major browser and compare the execution time.
What is the expected result?
The performance of this operation should be comparable on all the major browsers (IE, Edge, Firefox, Chrome).
What happens instead?
Chrome is 6 to 8 times slower than IE, Edge, Firefox.
Please provide any additional information below. Attach a screenshot if
possible.
,
May 6 2016
This is working as intended. That particular CL does indeed make this microbenchmark slower, but it does not make real-world code slower. It's very unlikely that you want to repeatedly set up the same prototype, which is what the CL makes slower. Now you're really measuring the cost of setting up a single object, rather than getting the benefit of already having it set up once and measuring something else subsequent times.
,
May 6 2016
It will make real-world code slower for anyone that uses Microsoft Ajax (which includes anyone using S#). I believe it will also impact users of TypeScript, because TS incrementally builds up the prototype object. If you look at http://www.typescriptlang.org/play/ the methods are sequentially assigned to the prototype object. I've attached another following the example at the type script URL. On my machine, Chrome takes 11s. Firefox takes 1s.
,
May 6 2016
Theb only thing it makes slower is setting up a second object that's identical to a first object. The performance of the first object is unchanged.
,
May 6 2016
I suspect I'm missing the point about the "second identical object"; I apologize if that's the case. I created another example to help illustrate: I have 1000 factory objects each returning a unique constructor. Each constructor's prototype is assigned by its unique factory 1000 unique methods in a loop. This is meant to simulate the way typescript lays out classes. Again, Chrome is an order of magnitude slower (12-15 seconds) than Firefox and Edge (1-2 seconds). Note, I created a third xample, locally, where classes are laid out completely declaritively, without the loop. I see the same thing (the file is MB in size, though, so I didn't upload it.) Finally, I want to reiterate that the original post is simulating the way MicrosoftAjax.js does inheritance -- for better or worse, it copies method's from a base's prototype onto the derived class. So, real world apps using MSAjax are impacted. And, if my TS example is valid, they will be as well. Thanks, twm
,
May 9 2016
Same thing. You are just hitting a cache that I removed in that particular CL. The cache was too expensive memory wise; and it only positively affected code such as the microbenchmarks you have created. It actually negatively affected using the prototypes later on. If you have an actual application that's measurably slower on the whole because of this though, let us know and we'll look into it.
,
May 10 2016
Yes, I do think our actual application is measurably slower because of this (as will be, I believe, any other application that uses MicrosoftAjax.js (or typescript).
We noticed a significant performance regression across "first run" scenarios that only impacted Chrome -- the initial page render, for example, and the first time a user initiated a particular action. Profiling showed Chrome was spending a lot of time in an MSAjax framework method called resolveInheritance.
The first time a class is new'd up in MicrosoftAjax, it needs to call resolveInheritance(). Whereas Firefox might spent 5ms in there, we see Chrome spending 50ms+. This is a one-time cost, but it significantly impacts the first-run scenarios.
The micro-benchmarks weren't designed to be synthetic, theoretical, benchmarks, but rather reduced repros of the problem area we are actually seeing in production.
Here is MicrosoftAjax's ResolveInheritance method. I've commented the line that's causing the trouble. Doesn't this suffer the same problem as the benchmark?
Type.prototype.resolveInheritance = function() {
if (this.__basePrototypePending) {
var t = this.__baseType;
t.resolveInheritance();
for (var n in t.prototype) {
var i = t.prototype[n];
this.prototype[n] || (this.prototype[n] = i) // THIS LINE
}
delete this.__basePrototypePending
}
};
Here is the typescript generated class from the link, above. Isn't the repeated assignment to Greeter.prototype going to cause the same problem?
Are you saying there is a difference between these use-cases and the benchmarks? If so, again, I apologize for not understanding the distinction.
var Greeter = (function () {
function Greeter(message) {
this.greeting = message;
}
Greeter.prototype.greet = function () {
return "Hello, " + this.greeting;
};
Greeter.prototype.meet = function () {
};
return Greeter;
}());
var greeter = new Greeter("world");
var button = document.createElement('button');
button.textContent = "Say Hello";
button.onclick = function () {
alert(greeter.greet());
};
document.body.appendChild(button);
,
May 10 2016
OK I see. In that case I'm marking this as a feature request because this has never been fast. Adjusting priority accordingly. Startup performance has high priority for us, so if this measurably affects startup we'll look into it. The bisect threw me off, and only pointed to my change since there's a difference between how we handle the provided microbenchmarks and the code you'd like to see fast.
,
Jun 1 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 2 2016
cc jkummerow@ who is exploring an idea to make this faster.
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/be0494ba5bfdc8f7b225372ed7990a06dca7ad46 commit be0494ba5bfdc8f7b225372ed7990a06dca7ad46 Author: jkummerow <jkummerow@chromium.org> Date: Wed Jun 08 14:43:22 2016 Keep prototype maps in dictionary mode until ICs see them Adding properties to prototypes is faster when we don't force their maps into fast mode yet. Once a prototype shows up in the IC system, its setup phase is likely over, and it makes sense to transition it to fast properties. This patch speeds up the microbenchmark in the bug by 20x. Octane-Typescript sees a 3% improvement. BUG= chromium:607010 Review-Url: https://codereview.chromium.org/2036493006 Cr-Commit-Position: refs/heads/master@{#36828} [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/api.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/builtins.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/crankshaft/hydrogen.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/factory.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/globals.h [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/ic/handler-compiler.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/ic/ic.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/keys.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/lookup.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/messages.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/objects-inl.h [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/objects.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/objects.h [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/prototype.h [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/runtime/runtime-array.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/runtime/runtime-debug.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/runtime/runtime-forin.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/src/string-stream.cc [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/test/mjsunit/dictionary-properties.js [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/test/mjsunit/fast-prototype.js [modify] https://crrev.com/be0494ba5bfdc8f7b225372ed7990a06dca7ad46/test/mjsunit/regress/regress-put-prototype-transition.js
,
Jun 10 2016
This should be fixed after #11. The caveat is that adding properties to prototypes is faster now *until* the prototype is actually "used" as a prototype. Example:
function MyObj() {...}
MyObj.prototype.method1 = ... // fast
MyObj.prototype.method2 = ... // fast
var obj = new MyObj();
for (...) {
// Depending on specific usage patterns, somewhere
// around here MyObj.prototype will transition from
// fast-add to fast-lookup mode.
obj.foo = ...
obj.method1();
}
MyObj.prototype.oops_I_forgot_something = ... // slow
Please report back if you're still seeing unexpected slowness.
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ajha@chromium.org
, May 5 2016Components: Blink>JavaScript
Labels: -Type-Bug -Pri-3 M-52 hasbisect OS-Linux OS-Mac OS-Windows Pri-1 Type-Bug-Regression
Owner: verwa...@chromium.org
Status: Assigned (was: Unconfirmed)