forked from davidtodd/landmarks
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Investigate performance of mutation observation #172
Labels
Comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On the UI Bootstrap page there is a carousel that causes Landmarks to constantly increase the scan pause timer. This is working well, but it's probably a good idea to see what's causing the mutations to be seen as potential landmark changes.
There's a possible next step, which is to assess if we're constantly getting consistently very many mutation events—of a certain type?—over a certain time period and, if so, disconnect the mutation observer (with the intent being that's temporary). Currently the mutation observer is never disconnected, hence the pause timer is used as a "userspace" solution to the throttling issue; Landmarks decides if/when it will investigate a mutation event or not. Performance seems to be fine, but it's good to have an opportunity to asses this. Further: if a page is totally inappropriate for constant scanning (such as a game?) perhaps this should be considered.
The problems with any situation in which the mutation observer may be disconnected are:
I was not able to reproduce #154 and I've no reason to think that performance is bad generally, but it would be good to get some decent profile data about this and actually check, now I've found a page that seems to offer that chance. It's worth testing it on a page that is a browser-based game too (and Google Docs, as it triggered the previous performance issues documented in #127). Relying on the speed of modern machines is not best practice (nor is obsessing over performance too much) :-).
There's also the elephant in the room: the possibility of re-writing the landmark detection to respond to mutation observers and work on small parts of the DOM, but that would be another complete re-write and thus very time-consuming and error-prone—and perhaps not even necessary as it should be possible to get great performance on pages that are appropriate, and maybe just auto-disable on pages that are not.
Finally, I don't like the idea of filtering out pages based on user preference lists—it adds burden on the user, and development, and pages can change. It would be much better to be able to detect, with good performance, if a page is suitable for Landmarks to run and, on a case-by-case basis.
Some things to consider:
The text was updated successfully, but these errors were encountered: