Fix Chrome performance regression #2960
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that dxSTable.rows is a getter, and not a variable, we need to be mindful about using it in for loops. On an ruTorrent with 20K torrents, putting it into the for loop, instead of copying it into a variable will result in 20K executions of the getter function and tank performance. By checking the rowcount once, before we start the loop, we can keep the getter (reducing the risk of OOS bugs), without suffering the performance penalty. Related to Novik#2830
|
I've added a second commit that further optimizes by making syncDOM() 5x faster the expense of removing the index attribute from all the tr's in the table. Looking at them, I believe the attribute lost all purpose it was originally intended for and might as well be cut, but feedback welcome. Let me know if you'd like these squashed. |
It looks like it has lost it's original purpose and is now identical on every single torrent when you first load it. It does look like it will increment as new torrents are adedd. But I don't see a lot of utliity. Nobody needs to reference the 57th torrent added to the client. It's also incurring a hefty performance penalty in the code. Removing it now makes syncDom 5x faster. From 270 ms on my machine to 57 ms reliabily in Chrome.
|
Thanks @anthonyryan1. I will squash and merge. There's a button for that on GitHub. |
stickz
pushed a commit
that referenced
this pull request
Jun 18, 2025
* Fix Chrome performance regression Now that dxSTable.rows is a getter, and not a variable, we need to be mindful about using it in for loops. On an ruTorrent with 20K torrents, putting it into the for loop, instead of copying it into a variable will result in 20K executions of the getter function and tank performance. By checking the rowcount once, before we start the loop, we can keep the getter (reducing the risk of OOS bugs), without suffering the performance penalty. Related to #2830 * Remove tr index attribute It looks like it has lost it's original purpose and is now identical on every single torrent when you first load it. It does look like it will increment as new torrents are adedd. But I don't see a lot of utliity. Nobody needs to reference the 57th torrent added to the client. It's also incurring a hefty performance penalty in the code. Removing it now makes syncDom 5x faster. From 270 ms on my machine to 57 ms reliabily in Chrome.
stickz
added a commit
to stickz/docker-rtorrent-rutorrent
that referenced
this pull request
Jun 18, 2025
# ruTorrent v5.2.10 It is highly recommended for common platforms to upgrade. This hotfix resolves a performance regression in `v5.2` with dxSTable sorting. When running ten thousand torrents, there may be a significant slowdown without this hotfix. ## What's Changed * Fix Chrome performance regression by anthonyryan1 in Novik/ruTorrent#2960 * Refine pt-BR translation for country names by cantalupo555 in Novik/ruTorrent#2961 **Full Changelog**: Novik/ruTorrent@v5.2.9...v5.2.10
stickz
added a commit
to stickz/docker-rutorrent
that referenced
this pull request
Jun 18, 2025
# ruTorrent v5.2.10 It is highly recommended for common platforms to upgrade. This hotfix resolves a performance regression in `v5.2` with dxSTable sorting. When running ten thousand torrents, there may be a significant slowdown without this hotfix. ## What's Changed * Fix Chrome performance regression by anthonyryan1 in Novik/ruTorrent#2960 * Refine pt-BR translation for country names by cantalupo555 in Novik/ruTorrent#2961 **Full Changelog**: Novik/ruTorrent@v5.2.9...v5.2.10
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that dxSTable.rows is a getter, and not a variable, we need to be mindful about using it in for loops. On an ruTorrent with 20K torrents, putting it into the for loop, instead of copying it into a variable will result in 20K executions of the getter function and tank performance.
By checking the rowcount once, before we start the loop, we can keep the getter (reducing the risk of OOS bugs), without suffering the performance penalty.
Related to #2830