Add JS_TryGetProperty

[chqrlie]

• add wrapper around JS_HasProperty / JS_GetProperty
• further optimisation does not seem useful in the general case
• optimize JS_TryGetPropertyInt64 for fast arrays

[saghul]

IIRC Ben mentioned avoiding to walk the prototype chain twice. Is that still not worth pursuing?

[chqrlie]

IIRC Ben mentioned avoiding to walk the prototype chain twice. Is that still not worth pursuing?

As I wrote in the commit comment, avoiding the redundant prototype chain walk is left out for now because the use cases are not critical, except for array indexing, which I address separately in this patch.

Some Array methods should special case fast arrays: js_array_concat and js_array_lastIndexOf for example.

I am investigating 3 other ways to improve property access:

• JS_GetPropertyInternal could return a way for the caller to determine where the property was found if at all. It already updates the inline cache, so this would not add much overhead and would fully address the purpose of JS_TryGetProperty.
• We could maintain shape flags to track the presence of getters and/or setters except for __proto__ in the prototype chain. This would simplify JS_SetProperty and remove the need to walk the prototype chain. This requires a tricky change in the prototype chain handling, where a modification of a prototype object shape can be efficiently reflected in all prototype shapes that have a given shape as their own prototype.
• With this modification in place, a much more efficient inline cache for property access can be implemented that would also cache property access in the prototype chain, hence optimize method calls.

[bnoordhuis]

It might make sense to put these together next to the typed array constants inside the enum so they make up a dense range.

[chqrlie]

That's a good point, but it justifies a separate commit as changing the class ID order has subtle implications, among which all array initializers that use class IDs as index values and possibly other ones too.

[bnoordhuis]

I don't think this is correct, although the problem is probably more with JS_HasProperty.

JS_GetProperty(obj, prop) with obj="bar" and prop=1 returns "a", whereas JS_HasProperty claims the property doesn't exist.

In a similar vein, obj="foo" and prop="length" should produce 3, not property-not-found.

[chqrlie]

I agree, JS_HasProperty ignores properties of values that are not objects, which is the semantics of ECMA 7.3.12 HasProperty ( O, P ) that requires O to be an Object.

Similarly, JS_TryGetProperty() and JS_TryGetPropertyInt64() must only be called for Objects. These APIs should probably take a JSObject * but changing that would have caused too many modifications.

[chqrlie]

I am not sure JS_TryGetProperty() is useful after all. Optimizing JS_TryGetPropertyInt64() is definitely a good idea for array methods, but a more general function does not seem to provide a real advantage and may create some confusion. I shall investigate some more and post and updated patch.

[bnoordhuis]

My main motivation for #105 is to avoid looking up the property twice. In that respect a JS_TryGetProperty that's just JS_HasProperty + JS_GetProperty without caching the lookup indeed isn't useful.

[chqrlie]

A better approach to avoiding the double lookup is to use an alternate JS_GetPropertyInternal function that would update a structure argument with the location of the property found: prototype level, fast_array / shape entry, getter?, exotic method call...

[bnoordhuis]

Yes, that's pretty much what I was thinking.

[chqrlie]

OK, I shall simplify this PR and keep only the JS_TryGetPropertyInt64.