Skip to content
This repository has been archived by the owner on Apr 4, 2022. It is now read-only.

Investigate alternatives to MutationObserver for attributeChanged detection #19

Closed
benjamind opened this issue Jan 18, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@benjamind
Copy link

MutationObserver is potentially expensive if we have many elements on the page, especially if there are faster alternatives. Web Components have a built in mechanism for attribute change detection but it is driven from the observedAttributes static get method.

To use this mechanism we would need a list of all properties of ThreeJS classes at the point we declare the class for each of the matching custom elements. This class is declared in the ThreeElement.for static function. Unfortunately due to the way ThreeJS implementation is constructed we cannot enumerate the properties of the class until after instantiation of an instance of it since many of the class types only define their properties in their constructors.

I spent a little time today investigating potential approaches, given that observedAttributes is evaluated at custom element registration time and we cannot re-trigger this evaluation I have had to rule out a number of different options, this leaves the following potential paths as far as I can see:

Continue using MutationObserver
This probably needs profiling to determine if its acceptable on a scene with many objects, I suspect in a large scene this will be a problem.

Instantiate an instance of each class
In this approach we would instantiate each class and then enumerate the object instances own properties. There are a few downsides with this approach in that some of the ThreeJS apis require parameters to be passed in order to cleanly construct, this will then require a bunch of special case code to handle these dummy instances as well as unwinding any sideeffects creating these dummies would cause.

Use typescript to preprocess the ThreeJS types
This would essentially be a build step that could use the TS compiler API to load the types for the ThreeJS api and build a map of class name to attributes. The TS types fully define the interface to the API and this would then allow us to return the appropriate static list of attributes in the observedAttributes implementation. Downside of course is this requires a build step, and will need to be kept in lcckstep with any Three api changes. Performance however would be good, with a slight payload cost to enumerate all these types and attributes.

@benjamind
Copy link
Author

I see @hmans has already raised this issue on the webcomponents WICG here

@benjamind benjamind changed the title Investigate alternatives to MutationObserver for attributedChanged detection Investigate alternatives to MutationObserver for attributedhanged detection Jan 18, 2021
@benjamind benjamind changed the title Investigate alternatives to MutationObserver for attributedhanged detection Investigate alternatives to MutationObserver for attributeChanged detection Jan 18, 2021
@hmans
Copy link
Owner

hmans commented Jan 18, 2021

Hi @benjamind and thank you very much for opening this issue and going into so much detail.

That fact that three-elements is going through MutationObserver to mitigate the observedAttributes "issue" is definitely something that I'm not happy with, and needs to be addressed at some point. Right now, though, I simply don't have any data to work with; as you have already mentioned, it is something that needs to be measured and benchmarked in a larger project. (Luckily, I'm working on a larger project, so that will give me plenty of opportunity to do this!)

I've thought (quite a lot) about all the options you've described, plus one that's a bit crazy where, after inserting elements into the DOM, they immediately replace themselves with different, "upgraded" elements that have their observedAttributes set to the properties of the wrapped object. I'm confident it would work, but I believe the impact it would have on the overall approachability of the library would be catastrophic.

So, here's how I've been planning on tackling this:

In the long term, I'm hoping that there will either be improvements coming in a future version of the WebComponents spec (just being able to imperatively update observedAttributes, as I very naively suggested in the WICG discussion, would be a boon), or someone much smarter than myself will figure out a cool workaround. :-)

In the medium term, there are opportunities to allow authors of complex projects to selectively optimize this. I haven't spent any time documenting this, but three-elements already allows you to generate new tags backed by ThreeElement.for(constructor). We could allow users to pass a list of attributes there that will become the class' observedAttributes. When -- if -- users are seeing bottlenecks around the use of MutationObserver, they could use this to manually optimize things.

In the short term I'm actually relatively confident that this isn't going to be a big issue, for the simple reason that in most non-trivial projects, you will end up mutating the Three.js object directly instead of going through its element's attributes. This of course largely depends on the code architecture you're choosing and the framework you're coupling three-elements with, and at this point, we don't really know yet what patterns are going to emerge here, but just like in react-three-fiber, where you're advised to mutate the Three.js objects directly instead of changing props (and causing re-rendering of the component tree), a similar rule of thumb can be established here.

(Ironically, the way that I'm using three-elements in my big project heavily relies on changing elements' attributes, so if there is a problem, I will be among the first people to be hit by it; anyone reading this can rest assured that I will have enough reasons to stay on top of this topic. :D)

@hmans hmans added the enhancement New feature or request label Jan 18, 2021
@hmans
Copy link
Owner

hmans commented Jan 26, 2021

Here's a PR with a potential solution: #34

@hmans hmans closed this as completed in e74ca07 Jan 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants