Proposed web performance improvement: unload the content upon losing focus

Tiny Tiny RSS Development
1

First post here.

I have been using ttrss for a few years, so really appreciate the effort into this.

There is one thing bugging me for a while. If I have a lot (say >100) headlines in a feed, and I am reading them one by one, the tab will get slower over time. I noticed that the read headlines never recycle their content container. I wrote a proof of concept, and it seems to be working pretty well for me.

        var origSetActive = Article.setActive;
        Article.setActive = function(id) {
            var activeId = Article.getActive();
            if (activeId && activeId != id) {
                const row = $('RROW-' + activeId);
                if (row && !row.hasAttribute('data-content')) {
                    const container = row.querySelector('.content-inner');
                    row.setAttribute('data-content', container.innerHTML);
                    container.innerHTML = '';
                }
            }
            return origSetActive(id);
        }

Obviously, more works are needed for it to work correctly (using the original data-content, not processed). We can also unload lazily, only the least recently used article gets evicted.

Suggestions?

2

i’m not sure what you’re trying to accomplish with this. you’re assigning live content back to the attribute, even though on every article selection it is replaced entirely anyway?

also, you seemingly haven’t noticed App.cleanupMemory() being called in Article.render() which cleans up all dojo objects in content node recursively?

e: also, there’s nothing in here about losing any kind of focus?

3

i’m running with profiler right now and i can tell you one thing, i don’t think App.cleanupMemory() actually does anything of value. profiler graph with and without it seems almost entirely equal.

e: this was originally added, if i remember right, because of dojo-specific leaks. what this could mean is that dojo since then stopped leaking on dijit content assignment.

workload being “open a feed and go through a hundred or so articles”.

still, you’re trying to do things with content-inner which is recycled entirely on article switch. everything below content-insert (which is the host dijit for all post-related things) is reassigned.

4

This code is only a proof of concept, so I recognize it’s very very far from good. It assumes “Always expand articles” being off, therefore only one article is expanded (which of course needs to be fixed).

My intention is to remove the article content from DOM once it is collapsed. If each article has a video tag in it, it will consumes a lot of resources by simply leaving them in DOM, as compared to putting them back as a string. They add up pretty quickly.

5

ah, i understand better now. i thought this was about three panel mode.

non-expanded combined mode is a bit of a redheaded stepchild in tt-rss. i tend to forget it exists.

6

Thanks for the quick reply! Ideally it should unload the DOM content once an article is un-collapsed. Does that make sense to you?

7

i think a cleaner way would be doing it like this:

diff --git a/js/Article.js b/js/Article.js
index b9f35c706..43fe19d24 100644
--- a/js/Article.js
+++ b/js/Article.js
@@ -178,6 +178,10 @@ define(["dojo/_base/declare"], function (declare) {
                                if (container.textContent.length == 0)
                                        container.innerHTML += " ";

+                               // in expandable mode, save content for later, so that we can pack unfocused rows back
+                               if (App.isCombinedMode() && $("main").hasClassName("expandable"))
+                                       row.setAttribute("data-content-original", row.getAttribute("data-content"));
+
                                row.removeAttribute("data-content");

                                PluginHost.run(PluginHost.HOOK_ARTICLE_RENDERED_CDM, row);
@@ -308,6 +312,11 @@ define(["dojo/_base/declare"], function (declare) {
                        console.log("setActive", id);

                        $$("div[id*=RROW][class*=active]").each((e) => {
+                               if (e.hasAttribute("data-content-original")) {
+                                       console.log("packing", e.id);
+                                       e.setAttribute("data-content", e.getAttribute("data-content-original"));
+                                       e.innerHTML = "";
+                               }
                                e.removeClassName("active");
                        });
8

Looks great. Yeah using the original payload is the correct behavior here.

9

https://git.tt-rss.org/fox/tt-rss/commit/b3e4f0188e68ee7f61121b247d7202fb68d9c28e