-
-
Notifications
You must be signed in to change notification settings - Fork 552
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
[special performance][upwards compatibility] Construct our (combined) mutation observer/s individually depending settings? #2276
Comments
youtube/js&css/web-accessible/init.js Line 68 in 96879c6
this never executes because its being called before storage loads. This is when you broke it c7695ff#r142127013 This mutation observer listens (well, listened until you broke it :P) to a very specific thing Combining all mutation observers into one colossus targeting document.documentElement is a bad idea :( it triggers on everything.
Currently on channel page ytElementsHandler runs ~2000 times before first thumbnail is even inserted into the page, and keeps running for another ~20000 times before last thumbnail is added. Just to drive point home adding
here youtube/js&css/web-accessible/functions.js Line 269 in 96879c6
gives us ~24000 times ytElementsHandler was run without any reason, + another 12000 it matched youtube/js&css/web-accessible/functions.js Line 252 in 96879c6
... then again all this pointless calling of ytElementsHandler combined steals "only" ~30ms for average Video page, thats two frames of screen refresh, not the end of the world in html land. In fact Instead of combining its the other way around. Optimally there should be multiple individual mutation observers listening only on specific target elements. I think this might be what you meant by "construct". So instead of youtube/js&css/web-accessible/functions.js Lines 40 to 44 in 96879c6
testing every link for 'ytd-thumbnail' class to run blocklist there would be dedicated mutation observer added only after '#items #contents' suggestion sidebar was added to the page and listening to only changes inside '#items #contents' target. Problem with this solution is fragility, every time YT does the slightest change (order it injects content) things could/would break. The lightest way possible is waiting until YT loads everything and then scanning once for things we are interested in, this should handle 90% of what ImprovedTube.ytElementsHandler is scanning for on every element on every mutation. Only after this one big scan would ytElementsHandler mutation observer be started. |
hi! @raszpl!
-and avoid depth of recursion at first, unless it will be of interest somewhere later/cheaper Yes, to be (much) future-proof would be worth some ms 😊 (/not hard to have the confidence to add 0.5% to the total time YouTube loads anyways.), yet we aren't using the Observer much like that and these edits making guesses should be aware how often these lines might run: https://github.com/code-charity/youtube/blob/96879c6e2ce0321d68b1a27f36251c9a92beba44/js%26css/web-accessible/functions.js#L171-220 youtube/js&css/web-accessible/functions.js Lines 72 to 107 in 96879c6
And it doesn't watch attributes & characterData yet. But one could suppress many elements instantly to clean, ( thats after adjusting JS before DOM is ready or after intercepting http request with firefox w3c/webextensions#506) )
channel_default_tab() is working 😅
blockable items are also called #dismissible. In #secondary #related, ytd-search, ytd-two-column-browse-results-renderer, and the related videos grid ( repo-history 2020 https://github.com/code-charity/youtube/blob/2fc9341cb2066240a3b6dfc2da763feb06dec5e1/src/youtube/js/mutations.js 2021 https://raw.githubusercontent.com/code-charity/youtube/2bba2fe772dc8a7abdc06cfe8475bc88b6c5ed56/youtube-scripts.js https://raw.githubusercontent.com/code-charity/youtube/2bba2fe772dc8a7abdc06cfe8475bc88b6c5ed56/content-scripts.js ) |
that function is so broken its not even funny, what is this clown car of a try catch chain? :) the first one will always match because every node this deep has 4 childnodes :-] 2 makes some sense but is bad because slow 3 makes zero sense
Speaking of Playlists, Playlist menu has those 4:
but: I have this semi fixed locally together with some optimizations I will be submitting later.
You might think you can, but no:
because its also being called in
making this whole another separate mutation observer do useless double work :(
after running some monitoring blocklist doenst cost us much, YT likes to add thumbnail A nodes and then keep reusing them over and over
Hey this looks familiar
its my #2197 :] |
...A quick test from when the line looking for four caused an error. |
will you / we? |
"Features can have their own mutation observer each" (like this:)
youtube/js&css/web-accessible/init.js
Lines 67 to 75 in 96879c6
And we have one combined observer processing the DOM once, and then also catching possible unexpected changes of the DOM since maybe features need to run again depending what element might change by youtube's js. So this runs a lot even without checking changes of attributes and content. (Yet even a combined observer could be constructed individually depending user settings. The average user might has only 10 in 180 features enabled. )
youtube/js&css/web-accessible/init.js
Lines 8 to 22 in 96879c6
youtube/js&css/web-accessible/functions.js
Lines 39 to 43 in 96879c6
( the commented out code functions.js 20-33 & init.js 30-54 are just a little optimization to be tested once more)
The text was updated successfully, but these errors were encountered: