Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refresh optimization #179

Open
JoernT opened this issue Dec 3, 2022 · 7 comments
Open

refresh optimization #179

JoernT opened this issue Dec 3, 2022 · 7 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request epic

Comments

@JoernT
Copy link
Contributor

JoernT commented Dec 3, 2022

A new approach to refresh using:

  • mutation observers,
  • intersection observers,
  • and model change information

in combination to provide optimum refresh speed on page load and during runtime.

@JoernT JoernT added documentation Improvements or additions to documentation enhancement New feature or request epic labels Dec 3, 2022
@JoernT
Copy link
Contributor Author

JoernT commented Dec 3, 2022

Please @DrRataplan share some thoughts/documentation about the conceptual underpinnings of the new refresh approach.

@DrRataplan
Copy link
Collaborator

DrRataplan commented Dec 13, 2022

The premise is simple. I wrote a paper for XML Prague outlining the approach. This can be found at https://www.fontoxml.com/wp-content/uploads/xmlprague-2017-proceedings-soft-validation-in-an-editor-environment.pdf?_gl=1*l1fvk0*_ga*MTcwNzIwNTMwLjE2NzA5MzU5MTY.*_up*MQ..

It's a hard read, so I'll summarize here.

Problem

Refreshes are costly. Any component that refreshes needs to execute a number of XPaths. For example, in a fx-repeat, the XPath for the repetition is executed. We should always prevent 'useless' refreshes: those that will result in the same end state.

Example

Take an application with two repeats. One over countries and one over provinces. They are separate and next to each other. The XML is two lists: one of countries, one of provinces:

<table>
<countries>
<country code="NL">The Netherlands</country>
<country code="SE">Sweden</country>
<country code="DE">Germany</country>
</countries
<provinces>
<province country="NL">Zuid Holland</province>
<province country="SE">Skåne</province>
<province country="DE">Bayern</province>
</provinces>

<focusedCountry>SE</focusedCountry>
</table>

The countries repeat just repeats over country/countries, but the provinces uses a more complex query: provinces/province[@country=/table/focusedCountry]. When selecting the country from the first repeat, the focusedCountry element is updated and the provinces repeat should update.

So far, it's ok. But now imagine we are going to add new countries. If we add a country, the provinces should not need to be refreshed. Same for adding a province. Only when adding a new province with a country that matches, do we have a reasonable refresh for the province repeat.

If we are sure nothing changed that the XPath touched, the refresh will be useless. Only if changing any of the 'things' the XPath touches we might need an update.

Imagine both lists are big; full refreshes take time.

Solution

We can use the mutationObserver API to get hold of any changes to a piece of XML.

This API gives out info like

  • type: 'attributes', attributeName: 'code', target: <country />
  • or type: 'childList', addedNodes: [<country code="BE">Belgium</country>], target: <countries/>
  • or type: 'data', oldValue: 'NL', target: text {SE}

With this, we can see what happened when in the XML.

Linking that to the XPath is where a domFacade comes in. This is an API with FontoXPath that intercepts all DOM access in an XPath. In an XPath like province/provinces[@country=/table/focusedCountry], the DOM access will be something like this:

[
childList on <province/> // To iterate over provinces
attributes on <province country="SE"> // To compare the country attribute
attributes on <province country="DE"> // Same
attributes on <province country="NL">// Same
childList on <table/> // for the `/table/focusedCountry` part of the path
childList on <focusedCountry/> //For the atomization of this element, for the compare
data on  text {SE} // To read the data of the textnode
]

Repeats

We now have a bug with repeats: if we do not do full refreshes all the time, we have too few. This is because a repeat may not always have a model item bound to it. Especially with complexer XMLs, this can be the case.

If we can get hold of the dependencies of a repeat, we can minimize the refreshes. At least prevent refreshes on repeats that only handle XML that is totally unrelated to the last action by the author.

Hope this makes a bit of sense. I'll update it when I get further under way with the refactoring

@JoernT
Copy link
Contributor Author

JoernT commented Dec 14, 2022

just formatting

@JoernT
Copy link
Contributor Author

JoernT commented Dec 15, 2022

fine with that so far. Next step would be to add some description of the algorithm and its integration into current code when going along.

@JoernT
Copy link
Contributor Author

JoernT commented Dec 16, 2022

As we already discussed once we got the mutationobservers in place we can top it up by 'filtering' the dirty list with intersectionobserver, so that only items get refreshed that are actually in view.

Main advantage of using intersectionobserver is that is narrows down the amount of refresh calls for initial page load when it's a long scrolling page.

However intersectionobserver will also have an impact on how mutationobservers are setup and therefore should be at least considered in the intial design. Avoiding any cost for elements not being displayed would deny a full page scan for mutationobservers? Thus these would rather follow the visible 'frame' top-down (when the user scrolls the page) and add up incrementally. What do you think @DrRataplan ?

To not forget about accessibility: what happens in a screenreader which obviously has no 'visible' part of the UI. Intersectionobserver won't help much here.

@JoernT
Copy link
Contributor Author

JoernT commented Jan 2, 2023

update: later further research revealed that the actual DOM element creation is not the main factor as it's only taking a fraction of the time of the layout/paint followed by that.

There a new CSS kid in town called 'content-visitbility' that shall help with the problem but i couldn't yet find any impact with that or failed applying it in a manner that causes effect.

@DrRataplan
Copy link
Collaborator

My understanding is that the paint / layouting that is hurting us, it is rather the searching for what to update. Big part of this is the template expressions. That is one of the last places where we do a blind unoptimized traversal to find all template expressions. And after that, we update all of them, one by one.

Talking about this traversal: https://github.com/Jinntec/Fore/blob/dev/src/fx-fore.js#L462, and this update loop: https://github.com/Jinntec/Fore/blob/dev/src/fx-fore.js#L494

In here the mutation observers should help out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request epic
Projects
Status: In Progress
Development

No branches or pull requests

2 participants