Faster resolution of interceptors for @JS() classes

[greglittlefield-wf]

It would be awesome if interceptor resolution were faster for JS objects accessed in Dart via interop classes using the js package's @JS annotation.

Background

I'm working on improving the performance of react-dart, a Dart wrapper for the React JS library.

I recently upgraded the library to use new js package interop, which resulted in significant performance improvements. Go, js package!! 🎉

While testing performance, I noticed that lookupAndCacheInterceptor was taking a significant amount of time.

react-dart hooks into React JS's rendering via allowInteropCaptureThis, and unfortunately ends up having to access a large amount of JS objects (the JS this and its JS properties) during each rendering cycle, which means lots of JS object interceptor resolution.

Idea

One thought I had would be, in lookupAndCacheInterceptor, to resolve to UnknownJavaScriptObject when an object's tag (constructor name) matches the a parameter passed into the @JS annotation.

However, that'd cause issues if that tag wasn't unique... Thought I'd throw out the idea, though.

[rakudrama]

The problem is that we have to be conservative and not cache the results on an object when we don't know the structure of the object. That leads to repeated calls to lookupAnd(MaybeNot)CacheInterceptor. If we knew for sure there was a constructor function and associated prototype chain, and that things were nicely behaved, we could cache once on the prototype (this is what we do for e.g. most Html Elements). We still would not be able to cache on objects created with object literals like {foo: function(){...}}.

The compiler shares lookups when it can see they are on the same (unmodified) variable. Your code is pretty good at holding re-used values in local variables. Unfortunately we can't yet share the lookups across functions.

You can also try compiling with '--trust-type-annotations'. It is a bit of a sharp knife and if anything is misbehaving, the first step is to try without '--trust-type-annotations', and possibly '--checked' to see if the types are in fact lies.

I don't have any other suggestions yet, but a small change will help tell which closure is which in the profile. You can always name a closure by defining a local function instead of an anonymous closure.

    var redraw = () {
      if (internal.isMounted) {
        jsThis.setState(emptyJsMap);
      }
    };
-->
    redraw() {
      if (internal.isMounted) {
        jsThis.setState(emptyJsMap);
      }
    };

[greglittlefield-wf]

So you're saying that caching based on an unknown constructors and prototype chains involves too many unknowns to always work properly? That makes sense...

Also, a fair amount of the interoped objects react-dart is dealing with (e.g., JS props maps) are plain JS objects with no prototypes, so caching wouldn't be able to help the performance of those—the lookup would have to be faster (not that that's possible).

More ideas (just spitballing, here):

• Would it be possible for a consumer to somehow mark those prototypes manually as "safe for Dart interceptor caching"?
• Could we allow for a custom tag function to be defined on the object, which could aid in uncacheable lookups?

---

Okay, cool, I'll do some further testing with --trust-type-annotations and local functions.

Thanks for the explanations and advice!!

[greglittlefield-wf]

Follow-up: for the specific problem I was working on, I decided to go the route of writing some custom JS to interact with the JS objects I'm dealing with, which avoids Dart code from looking up interceptors. See: https://github.com/cleandart/react-dart/pull/95.

These changes merely sidestep the issue; it'd still be nice to not have to worry about interceptor overhead.