Fix Shadow DOM performance issue and restore its support #3365
Conversation
|
Thank you @heiwiper! I want you to know that this PR is not unnoticed! |
|
Thanks for the followup! I totally understand, especially considering that the related issue is marked as low priority. |
|
@heiwiper I've tested this branch and I think the results are amazing! I took the richest URL in terms of hints (~8k) and I can't notice any difference between this branch and master. That means that shadow DOM support adds no overhead, as intended. And the feature is working too. I've tested it with https://developer.servicenow.com/dev.do. In short, I think it's mostly done. Please mark it as ready and add a changelog entry. We'll integrate these changes in the next release in the 3 series. Thanks! |
Avoids iterating through all DOM nodes to walk through shadow DOM elements. Instead, we make use of the nyxt class 'nyxt-shadow-dom'. Note that the use of 'do' loop macro makes sure we get that parenscript generates the desired code in javascript.
Unlike the macro 'rqs-nyxt-id', we don't make use of 'nyxt-shadow-root' attribute because the macro 'rqsa' might get called befoore the DOM is update through the function 'update-document-model'. We use the TreeWalker for this algorithm based on the benchmarks of the following StackOverflow response https://stackoverflow.com/a/64551276 Ensure that the results produced by rqsa are sorted in depth first algorithm so that it matches the generated hints order which is produced in the same order according to the doocumentation of the function 'clss:select'. Note that sorting the output array based on 'nyxt-identifier' is not possible because elements that are dynamically added to the DOM are assigned a 'nyxt-identifier' that is greater than the last generated identifier, so it would break the depth-first order. Add to that the fact that 'rqsa' macro might be called on elements that do not have nyxt-attributes as mentioned previously.
heiwiper
force-pushed
the
shadow-hinting-performance
branch
from
4556216 to
94459c0
Compare
|
I have just added the changelog entry and rebased into master. Thanks for taking the time to test these changes! |
|
Thanks again @heiwiper. |
|
My pleasure! Feel free to tag me in related issues. |
Description
The algorithm for the macro
rqsahas been improved by using Tree Walker instead of Node Iterator and the matching elements are now sorted in depth first order. The sorting order was necessary to fix the performance issue, You can read more about the details of that issue in the references issues below.The algorithm for the macro
rqs-nyxt-idhas been improved by directly retrieving Shadow root elements if the matching element cannot be found by executingquerySelectfunction. Previously, we would walk through all elements to check whether each element is a Shadow root. Please note that we can't use this same method in therqsamacro because the that macro might be executed on elements that do not yet have nyxt attributes assigned which isn't the case forrqs-nyxt-id.Fixes #3295
Checklist:
(asdf:test-system :nyxt/<renderer>))