possible excessive proxy invariants for Object.keys/etc??
allen at wirfs-brock.com
Mon Nov 19 14:11:52 PST 2012
On Nov 19, 2012, at 10:04 AM, Tom Van Cutsem wrote:
> Hi Allen,
> 2012/11/18 Allen Wirfs-Brock <allen at wirfs-brock.com>
> The proxy spec.for Object.getOwnPropertyNames/kets/etc. seem to be doing quite a bit more than this. They
> 1) always copy the array returned from the trap? Why is this necessary? Sure the author of a trap should probably always return a fresh object but not doing so doesn't violate the integrity of the frozen/sealed invariants? In most cases they will provide a fresh object and copying adds unnecessary work that is proportional to the number of names to every such call.
> The copying is to ensure:
> a) that the result is an Array
Why is Array-ness essential? It was what ES5 specified, but ES5 had a fixed implementations of Object.getOwnPropertyNames/keys so that requirement was simply a reflection of what the spec's algorithms actually did. Most places in the specification that consume an "array" only require array-like-ness, not an actual array. Now that we're make the implementation of the getOPN/keys extensible, it would be natural to relax the requirement that they produce a [[Class]]==="Array" object.
> b) that all the elements of the result are Strings
And presumably Symbols. We have to also accommodate Symbols, at least for getOwnPropetyNames. Regardless, why is this important? More below...
> c) to ensure the stability of the result.
> You can think of a + b as implementing a type coercion of the trap result to "Array of String". This coercion is not too dissimilar from what the getOwnPropertyDescriptor has to do (normalization of the returned property descriptor by creating a fresh copy).
Yes, premature type coercion, in my opinion. Also, a classic performance mistakes made by dynamic language programmers: unnecessary coercion of values that are never going to be access or redundant coercion checks of values that are already of the property type. Why is it important to do such checks on values that are are just passing through these traps. When and if somebody actually gets arounds to using one of the elements that are returned as a property key they will be automatically coerced to a valid value. Why is it important that it happens any sooner than that.
getOwnPropertyDescriptor is also on my list to talk to you about for similar reasons. I don't see why the property descriptor needs to be normalized and regenerated (including converting accessor properties on the descriptor into data properties and requiring the [[Prototype]] to be object. I have an alternative for [[GetOwnProperty]]/TrapGetOwnPropertyDescriptor that preserves the original object returned from the trap (while normalizing to a descriptor record for any internal calls on ordinary objects). This preserves any exotic property descriptor object properties as is. This seems like exactly the right things. Such exotic property descriptor properties can only be meaningfully used by other proxy traps or application/library code that knows about them. I don't see any need to normalize them into data properties or otherwise muck with what the actually implementor of the getOwnPropertyDescrptor trap choose to return.
I this alternative implementation is, again, much simpler (and efficient) with fewer externally observable calls in its algorithm steps. I sure you will want to review it so I will include it in the spec. draft.
> c) on the other hand is crucial for the non-configurability/non-extensibility checks mentioned below. It's no use checking some invariants on a data structure if that data structure can later still be mutated.
I don't see how an extra copy of the the result array makes any difference in this regard (also see below). The array returned from Object.getOwnPropertyNames is still mutable even after you make a copy. It's only as stable as any other non-frozen object you are likely to encounter.
> If we don't care about any of a, b and c, then the result array wouldn't need to be copied.
yes, I believe that should be the case.
> 2) ensuring that the list of property keys contains no duplicates. Why is this essential? Again, I don't see what it has to do with the integrity of the frozen/sealed invariants. It is extra and probably unnecessary work that is at least proportional to the number of names).
> We've been going back and forth over whether or not we wanted to prevent duplicates. I remember Andreas Gal being concerned about these kinds of issues (that was when he was doing the first Firefox prototype for old proxies, in the context of the enumerate() trap, which was called during a live for-in loop. IIRC, Firefox already did de-dupe checks, as properties already enumerated in a child object should not be re-visited when visiting a parent object)
> More recently, at the last TC39 meeting in Boston, we decided to change the return type of the enumerate() trap from Array[String] to Iterator, and in doing so waiving the duplicate properties check. Quoting from the "Open issues" section of <http://wiki.ecmascript.org/doku.php?id=harmony:direct_proxies#open_issues>:
> "Enumerate trap signature: consider making the enumerate() trap return an iterator rather than an array of strings. To retain the benefits of an iterator (no need to store collection in memory), we might need to waive the duplicate properties check. Resolution: accepted (duplicate properties check is waived in favor of iterator return type)"
> I guess if duplicate properties are not crucial for enumeration, they're also not crucial for Object.getOwnPropertyNames and Object.keys and can be dropped. Mark, can you comment?
> 3) Every name in the list returned by the trap code is looked up on the target to determine whether or not it exists, even if the target is extensible. Each of those lookup is observable (the target might itself be a proxy) so, according to the algorithm they all must be performed.
> This is where we get into actual checks required to enforce non-configurability/non-extensibility.
> Granted, the ES5 spec is not clear about the invariants on getOwnPropertyNames and keys. The currently specified invariants are a common-sense extrapolation of the existing invariants to cover these operations.
Yes, but if they aren't essential then we have to take into account the cost of doing non-essential invariant checks. These costs are both the actual runtime cost of doing the checks, the constraints upon implementation imposed by the need for complete and properly sequenced occurrence of all potentially externally observable algorithm steps, and finally (and least important) the added complexity of the specification which makes it harder to actually achieve interoperable implementations.
> In practice, it determines the degree of confidence that a programmer can have in Object.getOwnPropertyNames and friends when dealing with a frozen object. If we waive these invariant checks, then the result of those operations can never be trusted on to reliably introspect on a frozen object's list of property names:
> Object.isFrozen(proxy) // true
> Object.getOwnPropertyNames(proxy) // ['foo']
> Object.getOwnPropertyNames(proxy) // [ ]
> Here, the 'foo' property apparently disappeared on a frozen object.
Yes, but copying the result an extra time doesn't ensure that two separate calls produce the same result.
> If neither the for-in loop nor Object.getOwnPropertyNames nor Object.keys can reliably report an object's own properties, then we've made it impossible to reliably traverse and inspect a presumably deep-frozen object graph.
If that is the goal, that maybe what you need is a new operation that cannot be trapped. Your algorithm is essentially doing all the work, and more, necessary to generate an accurate report of the actual non-configurable properties of the target. If that is what is needed for your deep frozen traversals, add that operation. That seems better than allowing the Proxy trap to do the work to report anything it wants, but then coming in after the fact and checking whether what was report matches what you demand. In this case, it seems preferable to just provide the results you demand without any handler intervention.
This seems to harken back to the original proxy design that essentially turned frozen objects into regular objects with no handlers. If that is what you need, you should provide access to the results you need for the situations where you need them.
But you should not be adding this overhead for Proxy uses that don't have those requirements.
> 4) Every own property of the target, is observably looked up (possibly a second time) even if the object is extensible and has no non-configurable properties.
> We may be able to remove the redundancy of two lookups by restructuring the algorithm.
> There previously was some redundancy in other checks as well.
> It isn't clear to me if any of this work is really necessary to ensure integrity. After all, what can you do with any of these names other than use them as the property key argument to some other trap/internal method such as [[SetP]], [[DefineOwnProperty]], etc. Called on a proxy, those fundamental operations are going to enforce the integrity invariants of the actual properties involved so the get name checks doesn't really seem to be adding anything essential.
> Perhaps we can just get rid of all the above checking. It seems like a good idea to me.
> Alternatively, it suggests that a [[GetNonConfigurablePropertyNames]] internal method/trap would be a useful call to have as the integrity invariants only care about non-configurable properties. That would significantly limit the work in the case where there are none and limit the observable trap calls to only the non-configurable properties.
> That would be one way to speed things up.
> I share your concern that the current spec algorithm may be a bit too restrictive for implementations to allow optimizations. It's very imperative. We should ask the spec implementors to have a look at the draft spec and share their concerns.
Maybe we should move this discussion to es-discuss. What do you think?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the es-discuss