Your performance tests are incorrect

[kogarashisan]

Hello to all LikedIn development team!

I saw this issue: #17
but there is a reason to create another one. You indeed incorrectly mix object construction time with object creation and method calls, but proposed tests are also wrong.

It's not good to deceive people, but I do not insist that you replace the link in your README.
Just have a look at this better version of JS inheritance performance tests:

• Overridden method calls: http://jsperf.com/js-inheritance-method-calls/2
• Object construction: http://jsperf.com/js-inheritance-object-construction

Tests have their own repository with explanation of everything (this may also interest you): https://github.com/kogarashisan/PerfTests

@fresheneesz And here is the version, which includes "proto" from the referenced issue (you can see, that it's indeed slower):
http://jsperf.com/js-inheritance-method-calls/3

P.S. You may also have a look at my ClassManager: https://github.com/kogarashisan/ClassManager

[fresheneesz]

Hey @kogarashisan, looks like you know a lot about making accurate benchmarks. Thanks for making a test that uses proto, I may use that in place of the benchmarks I wrote. But I want to understand why you mix instance creation and method calls, tho? They seem like fundamentally different things - since some libraries may be faster at creating instances, but consequently slower at executing methods.

[fresheneesz]

Also it loks like your tests at http://jsperf.com/js-inheritance-method-calls/3 don't use all of the techniques you mentioned in https://github.com/kogarashisan/PerfTests . For example, you're using a counter that goes up to 100,000 and you don't have any code to prevent inlining. Was this intentional? I'm also wondering why you create a loop inside the test-case, since the test cases themselves are already executed iteratively.

[kogarashisan]

If you calculate the time to construct a pair of objects for any given library, and then divide it by the time of 100000 pairs of method calls for that library, you will get the error that is introduced by the object creation. For the fastest libraries, it's much less than 1% (I checked that with calculator), so can be neglected.

Another way would be to create a pair of objects for each inheritance model in preparation section,
and retrieve it before calling a method, like var instance1 = window.class_instances["Proto"].ChildA and then call it's method. In this case, loop is also desirable, cause scope lookup also takes some time, which is relatively big in comparison to method call.

So between these two ways, i have chosen mine, cause it creates many pairs of objects instead of one. If there was only one pair - then JS engine could introduce optimizations to polymorphic methods (they would be still polymorphic, but they could be a bit faster). For example, in "proto" methods are assigned in a clusure, so potentially it can be faster, if you test it against only one object. So one pair is good, but many pairs is better.

I'm not a benchmarking guru, that's not my specialization. These tests can be improved by adding a warm-up loop in the preparation section, in theory this can reduce deviation a bit...

For "proto" I'm using the same techniques as for all others:

• parent's method is really fat, so most likely it will not be inlined (I confess, that I did not check that for lack of time)
• all methods have cache_buster_xxxx variables to prevent V8 from sharing code with other tests (and to introduce equal slowdowns. Without these variables it would be faster)
• counter in range of 100000 is still an integer
• inner loop is needed to create more object instances. To my mind it's more reliable and closer to real-life usage.

If you have more questions - feel free to ask.

[fresheneesz]

Ohh, i see. The method being called has to be fat - not the contents of the loop - of course. Also, I was confused by counter in your https://github.com/kogarashisan/PerfTests description, because I recognized the i variable being used by the for loop as a "counter". But now i get what you meant.

So now I get everything except, why not separate object instantiation from method calling? While the object instantiation might be negligible in the test you wrote at http://jsperf.com/js-inheritance-method-calls/3 , it doesn't show you the performance of instantiation (which may be important in systems that create a LOT of objects). How would you go about that? Remove the method calls and move the instantiation into the loop?

[kogarashisan]

but... there is a separate test suite for that:
http://jsperf.com/js-inheritance-object-construction
Maybe I did no understand the question...

By the way, did you know about existence of https://github.com/ericelliott/stampit

[fresheneesz]

Ah i see. I still would pull out the object construction stuff unless it is required for a good performance test. Event if it is negligible, i'd feel better about the test. I haven't seen stampit before, but the methods aren't very well explained - not sure what the difference is between a lot of the methods (compose vs mixIn for example).

I created two additional revisions, one that removes the instance creation from the method call test:

http://jsperf.com/js-inheritance-method-calls/4

and a version of your object construction test that includes proto in the same way you included it in the other test:

http://jsperf.com/js-inheritance-object-construction/2

[kogarashisan]

Sorry, but seems like your tests may be wrong. For method calls you need a warm-up loop, like this:

var model_names = ["Proto", "TypeScript", "CMBrowserMono" ...]
model_names.forEach(function(name) {
    for (var _i = 0, _i < 10; _i++) {
        var instance1 = new window[name + "ChildA"]("a");
        var instance2 = new window[name + "ChildB"]("b");
        for (var i = 0; i < 50; i++) {
            instance1.method();
            instance2.method();
        }
    }
})

Second: i would make method calls of equal length. Some years ago, call to
CMServerPartialrefMonoChildinstanceA.method(); was slower, than ProtoinstanceA.method();.
Most likely, it will not matter for modern browsers, but to be sure:

var instance1 = TypeScriptinstanceA;
var instance2 = TypeScriptinstanceB;
for (i = 0; i < 100000; i++) {
    instance1.method();
...

P.S.
You could notice, that in your tests Proto is 25% faster than in mine. That means that test suites are at least different.

Also, I mentioned Stampit as example that you are inventing a wheel... your model looks similar. And when you assign methods in a closure - they become slower, see this

[fresheneesz]

I don't understand. I created my tests from your test: http://jsperf.com/js-inheritance-method-calls/3 . The only thing i changed there was to remove the instance creation in the individual tests (i moved them to the preparation code section). Where is the warm up code in your test?

Ah i see about stampit. Well i created proto for its interface, not to be fast. I'm happy that it has performance on par with many of these other fast inheritance libraries. There are also performance improvements I may implement without changing the basic way proto works.

[kogarashisan]

"Where is the warm up code in your test?" - in my tests it's not needed, cause they invoke method for multiple object instances, which they create. Your tests have just one instance of each subclass - as far as I understand, it's not enough to guarantee correctness (cause method can be optimized to work with these two instances).
It's hard for me to predict the consequences of such optimization. Maybe when you run one particular model test several times in a row - you will get different results, often very different.

P.S. I have just noticed, that my Native test produces different results, when you run it several times in a row. This means that my version is not so perfect, as I expected - numbers should be stable. I will try to consult someone who knows better.

[fresheneesz]

How's this? http://jsperf.com/js-inheritance-method-calls/5

And was there anything you could see wrong with this one: http://jsperf.com/js-inheritance-object-construction/2 ?

It'd be nice if we could agree on a set of tests to link to from each of our projects.

[kogarashisan]

http://jsperf.com/js-inheritance-object-construction/2 - seem okay.

http://jsperf.com/js-inheritance-method-calls/5 - Oh, my... this is very bad, but it's my fault :)
Native model gets somehow optimized, it should not be 3 times faster than anything else.

I know, that V8 constantly recompiles methods, that are actively used, but I don't know this process in depth. Your "ClassManager/Browser/Polymorphic" is faster then mine. It's just a theory, but seems like it's optimized despite existence of warm-up loop.

Looks like these tests need to be improved. Here is a suggestion: in preparation section create 5 pairs of objects, instead of one. And each model test should call method from five pairs of objects. Warm-up loop remains.

That's what I will try myself in a couple of days, when I have time. I will also study assembler output of each test to better understand what's happening.

[fresheneesz]

It looks like adding 5 method calls greatly reduced the lead Native had. I was too lazy to create new classes for each, but I alternated A and B for each to create 5 instances, and that seemed to work. Check it out here:

http://jsperf.com/js-inheritance-method-calls/6

[kogarashisan]

Improved quality of my tests, added a couple of models (not "proto") and explanation about Native.
I will not promote "proto", sorry: there is over 50 ... if not 70 of different class models, while only some of them are actually fast and usable. Please, stop creating your own class models.

[fresheneesz]

There's a lot of shitty class models out there, yes. You don't have to promote proto, but its api is the cleanest in my opinion, its one of the only ones that's compatible with other inheritance libraries, and seems like one of the fastest. I don't regret spending time making it.

[kogarashisan]

"but its api is the cleanest in my opinion" - no.
"its one of the only ones that's compatible with other inheritance libraries" - also no.
"seems like one of the fastest" - well, okay.

[kogarashisan]

I apologize. That was rude comment.

[fresheneesz]

I accept your apology. And what I meant that proto is "compatible with other inheritance libraries" is that you can build classes with proto that inherit from native objects, which includes objects generated from other inheritance libraries. I've had a hard time finding inheritance libraries that do that.