Refactor lazyGet

[mitchlloyd]

• Extract isCacheableDescriptor function
• Combine logic flow for when to use get vs use the meta cache

@mmun This has been fixed after your review.

[stefanpenner]

these have been left un-rolled because the run-time doesn't optimize these code-paths well. As they are invoked tens of thousands of times or more, this does make me nervous.

In reality, it may not make that much of a difference, but until the @eviltrout's awesome suite is more comprehensive I remain apprehensive.

[mitchlloyd]

@stefanpenner That's fine. I have one more of these that I'll throw at you guys that I think is uncontroversial and then leave it alone.

[stefanpenner]

lets wait for others feedback, like @krisselden and @ebryn

[mitchlloyd]

To possibly make quicker work of the last clean up PR that I had in mind:

These two methods appear to be identical and could be turned into one method, perhaps called notifyChange:
https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/chains.js#L299-L333

In a crash course on optimized code, @mmun told me that that sometimes two identical functions could allow the compiler to optimize two different code paths based on the arguments that are passed in. In this case I don't see any reason to believe these functions would be called with types of arguments (both are only called internally in this module).

Let me know if you think that would be a worthwhile change.

[krisselden]

@mitchlloyd this looks ok, but since this is only called with a not isCacheableDescriptor can you remove the not and make it isVolatileDescriptor.

volatile really should be renamed manual because just because you make a CP volatile doesn't other parts of the ember system from caching it if you say put it in a view, and manual just means it is up to you to notify to invalidate.

[mitchlloyd]

@krisselden What do you think about isVolatile instead of isVolatileDescriptor? The way this function would read:

function isVolatile(obj) {
    return !(isObject(obj) && obj.isDescriptor && obj._cacheable);
}

Passing something like null to isVolatileDescriptor and getting true back implies that null is a descriptor.

[krisselden]

sorry, I totally missed your response to my feedback, looks good