improve performance for template repeat over model objects with shared prototype

[jmesserly]

This fixes the case presented in: https://github.com/Polymer/polymer-dev/issues/101

What is happening there is we have a bunch of paths inside a <template repeat> such as {{model.path}}. With polymer-expressions, the model for each repeated item has the same __proto__, which is the containing element, and we don't want every property set there to ping every item in the list. This seems to be the simplest thing that prevents the bug.

(Note: if the property is deleted from rootObj, we should get a change record from rootObj which causes us to iterate all objects again, and at that point, will observe the prototype because it is no longer an ownProperty of rootObj. That won't happen for polymer-expression scopes, but could happen in general.)

[jmesserly]

I went ahead and rebased, so this CL can be tested without #73

[ajklein]

@jmesserly, do you have an intuition about how often this will avoid the prototype walk? At first glance, it looks to me like it should be super-common, but then it comes to mind that published properties are only present on the prototype...

I ask because I've been investigating the overhead involved in prototype observation, in general.

[jmesserly]

yeah, it wouldn't help for published props :(. The only case I know of that it fixes is the scopes created by PolymerExpressions+template repeat="{{model in data}}". In that case "model." is an own prop of each scope created for each repeated item. But at least in that example it reduces work per scroll to O(1) from O(N) where N==items in list.

On the other hand, if you used a filter or just a data property that came from the parent scope, you're out of luck :|

edit: clarity

[jmesserly]

Hmmm, thinking on this more, it seems like we could do this check always and not just for the root object (assuming #73 is included). In other words, if we found the property on a particular object, we'll observe that object, so why walk up to the prototype at all?

[sorvell]

If this turns out to be a significant optimization, we should be able to take advantage of it for Polymer's published properties. The same optimization could apply since published property values are stored at a private location on the element instance, but we'd need more fanciness to know we can use this code path.

[jmesserly]

This seemed to me like a helpful optimization, but waiting for an LGTM from someone before landing :)

[arv]

Do we need to handle the case where the own property gets deleted?

[jmesserly]

good call, I should at least have a test for that & mention it the comment. It should work because whenever it gets a change record (in callback below), it will rescan everything.

[rafaelw]

i think that will work since we'll run through this code path in that case again and object will no longer have the own-property -- but a test would be ideal.

[arv]

LGTM

[rafaelw]

lgtm w/ comment above.

[rafaelw]

actually, can you wait to merge until i push my construction benchmarks. i want to make sure the call to hasOwnProperty doesn't regress construction.

[jmesserly]

benchmarking sounds great to me. I could run it through https://github.com/Polymer/TemplateBinding/blob/master/benchmark/codereview-diff.html too.