search

turbulenz / turbulenz_engine

Turbulenz is a modular 3D and 2D game framework for making HTML5 powered games for browsers, desktops and mobile devices.
http://turbulenz.com
MIT License
4.59k stars 513 forks source link

Refactor all those cycles with .hasOwnProperty #34

Open ghost opened 10 years ago

ghost commented 10 years ago

It is the slowest way to get to enumerate own properties (http://jsperf.com/object-keysv-vs-object-hasownproperty/3).

If you argue with ECMAScript3 bacward compatibility, you aren't anyway (you use Object.defineProperty to define getters and setters in certain parts of the code), not to mention, it is hard to find non-ES5 browser nowadays...

davidgaleano commented 10 years ago

That is a good idea, I will give it a try.

davidgaleano commented 10 years ago

Interestingly, not calling hasOwnProperty at all is significantly faster in those cases where we control the prototype chain:

http://jsperf.com/object-keysv-vs-object-hasownproperty/5

I also changed the test to use less properties which is a more common situation.

ghost commented 10 years ago

It's brittle. And for-in is looked as deprecated, in ES6 the new for-of is promoted. The brittleness comes with the fact that if you change prototype chain for some reason, mysterious bug would appear if for-in is still there.

ghost commented 10 years ago

But the speedup is massive.

davidgaleano commented 10 years ago

As I said, in those cases where we totally control the prototype chain, not calling hasOwnProperty is a massive win, for any other situation I agree that using Object.keys seems like a better option. For the record, the Turbulenz Engine already does not call hasOwnProperty in a couple of cases where performance actually matters and we are confident that the prototype chain is safe.

Sometimes we actually want the key or both the key and the value (for..in or Object.keys), whilst sometimes we only want the value (for..of), but for..of does not seem to be available everywhere yet.

ghost commented 10 years ago

FYI, for..of gives you keys, values or both depending on which iterator you call it. But yes, it's not out there yet, ES6 is not yet finished.

David Galeano wrote:

As I said, in those cases where we totally control the prototype chain, not calling hasOwnProperty is a massive win, for any other situation I agree that using Object.keys seems like a better option. For the record, the Turbulenz Engine already does not call hasOwnProperty in a couple of cases where performance actually matters and we are confident that the prototype chain is /safe/.

Sometimes we actually want the key or both the key and the value (|for..in| or |Object.keys|), whilst sometimes we only want the value (|for..of|), but |for..of| does not seem to be available everywhere yet.

— Reply to this email directly or view it on GitHub https://github.com/turbulenz/turbulenz_engine/issues/34#issuecomment-39437258.

davidgaleano commented 10 years ago

AFAIK, for a regular object (which is what I thought we were talking about) for..of would give the property values:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

ghost commented 10 years ago

yes, yes, just, you can use k of obj.keys(), v of obj.values() or [k,v] of obj.itemsorhowisthispairiteratorcalled().

David Galeano wrote:

AFAIK, for a regular object (which is what I thought we were talking about) |for..of| would give the property values:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

— Reply to this email directly or view it on GitHub https://github.com/turbulenz/turbulenz_engine/issues/34#issuecomment-39446958.