Removes hasOwnProperty checks to improve overall performance.

[twokul]

Tested using a real application, it can be found here.

Fixes #2206.

normalizeAttributes

before:

after:

normalizeRelationships

before:

after the change, it doesn't get registered.

thanks to @stefanpenner for pointing it out.

[igorT]

Are we somehow making jquery return a hash with Object.create(null)? Otherwise I don't see how this works out? cc @stefanpenner

[sly7-7]

The problem with removing those checks is that the normalization will end up by adding undefined value to the hash, and we don't want that. see #2104. That beeing said, I don't know why the tests are passing... I must have missed something

[stefanpenner]

Then just guard against undefined. That code path is optimized in runtimes correctly. Has own prop is still crazy slow

[sly7-7]

@stefanpenner I'm not sure it's the same.
Consider a model such as

MyModel = DS.Model.extend({
  attr1: DS.attr(),
  attr2: DS.attr()
})

Imagine we want to store.normalize('myModel', {attr1: undefined}), the result should be {attr1: null}, eg, we want to normalize the corresponding attribute, only if present in the passed hash, and transform undefined to null.

[stefanpenner]

@sly7-7 we are reading from type.eachAttribute so we are already only enumerating properties that are defined via DS.attr. Currently we leave undefined, or convert undefined -> null. How about we just always convert to null. The perf difference is pretty significant, so finding a path that allows for this optimization seems pretty important. So lets brainstorm on a solution that gives an expected result, but maintains the performance checks.

in theory x in y could be optimized by runtimes to be just as fast as y[x] but currently that isn't the case... It could potentially even use the same cache that y[x] uses... Someday hopefully.

[sly7-7]

@stefanpenner I think always converting to null is not an option, this will not work for partial normalization because it would end up to replace all the currently defined attributes to null.
That beeing said, the perfs are dramatically impacted (just saw this jsperf, so this should definitely be fixed, but my brain does not help me finding a way to answer to "check if hash has key without using hasOwnProperty" 😿

[stefanpenner]

does partial materialization even make sense? Seems like a great way to introduce zalgo at the model layer.

[sly7-7]

The goal of #2104 was to make store.normalize beeing able to work with store.update.

To make this work, store.normalize should not add extra {key: null} pairs to the normalized hash. Maybe it was just a bad idea, but in that case, I think we have to find an other way to update a record.

[fivetanley]

@sly7-7 if memory serves me correctly I think at the last ember data meeting tomhuda and igor decided we're going to deprecate "update" and let people opt into merging behavior with their serializer.

cc @igorT @stefanpenner @tomdale @wycats

[sly7-7]

@fivetanley I don't know when was the last meeting, but if your're right, I think #2104 should be reverted, and #2193 should be closed. I just don't know if those comments were before or after the meeting.

[sly7-7]

@stefanpenner @igorT @fivetanley Was there any other discussions about those fu**** hasOwnProp ?

[igorT]

This seems related to #2472

[sly7-7]

@igorT Yeah, I talked with @wecc about that, and if we want his PR as this, then those checks are necessary.

[igorT]

I am not sure we can do this. We will end up cloberring data. Also, the perf difference seems like ~ 2%

[stefanpenner]

just remember 10 2% wins === 20% if you don't think its worth it, then thats your call. But I would encourage seeing what needs to change so we can avoid this as it merely compounds with other perf issues.

[igorT]

The only way I can think of is to change the serializer implementation to iterate over the provided hash vs iterating over record attrs. But then we have to use Object.keys, not sure whether that will be faster/slower. Also would break all the keyForAttribute methods people defined themselves

[stefanpenner]

@igor splitting the concept of patch vs general update is also a solution

[stefanpenner]

im not sure where you are seeing 2% as his benchmark was likely the result of a much larger experiment then a specialized store.push anyways, we can revisit if we need to. Bigger fish to fry, and maybe the browsers will have fixed/improved this..

[igorT]

@stefanpenner that would cause us to have double the serializer methods.