Optimize Container Tabs from Excessive Request Checks #8

Closed
opened 2025-04-21 15:12:31 -04:00 by idk · 13 comments
Owner

Currently the extension opts for an approach where every single request is checked for whether it should be delegated to a container tab, but this is not strictly necessary. If you're already inside of a container tab which belongs to an application, e.g. MuWire, then there is usually no need to check again(The exception being i2psnark, which can display web pages which can attempt to fetch resources from a server under attacker control). There may be other situations where it makes sense to limit the amount of checking that we do in firefox-default tabs but this is much more complicated(Fortunately, none of the other frequently-performed-actions are applied to firefox-default tabs so it may be less of an issue there,

Currently the extension opts for an approach where every single request is checked for whether it should be delegated to a container tab, but this is not strictly necessary. If you're already inside of a container tab which belongs to an application, e.g. MuWire, then there is usually no need to check again(The exception being i2psnark, which can display web pages which can attempt to fetch resources from a server under attacker control). There may be other situations where it makes sense to limit the amount of checking that we do in `firefox-default` tabs but this is much more complicated(Fortunately, none of the other frequently-performed-actions are applied to `firefox-default` tabs so it may be less of an issue there,
idk self-assigned this 2025-04-21 15:12:31 -04:00
idk closed this issue 2025-04-21 15:12:31 -04:00
Author
Owner

Yeah that tracks. Since localhost will never be involved with either x-i2p-location or x-i2p-torrentlocation I just excluded every localhost URL from that check. It's basically the last should-be-unnecessary for loop, webRequest.HttpHeaders is a plain array of objects {name: xxx, value: yyy, bindata: zzz} and I just loop over it to find the header I want and break as soon as I find it. I'm searching the array backward in non-localhost contexts now since most of the time the custom headers are at or near the end, but I don't think that's long-term way to speed this up for people, I'm going to need to find a better way to limit the impact of this check soon.

Yeah that tracks. Since localhost will never be involved with either x-i2p-location or x-i2p-torrentlocation I just excluded every localhost URL from that check. It's basically the last should-be-unnecessary for loop, webRequest.HttpHeaders is a plain array of objects `{name: xxx, value: yyy, bindata: zzz}` and I just loop over it to find the header I want and break as soon as I find it. I'm searching the array backward in non-localhost contexts now since most of the time the custom headers are at or near the end, but I don't think that's long-term way to speed this up for people, I'm going to need to find a better way to limit the impact of this check soon.
Author
Owner

Yes, now it's fast. Even the router console tab is faster.

Yes, now it's fast. Even the router console tab is faster.
Author
Owner

Also I should just re-factor and drastically simplify the header setup.

Also I should just re-factor and drastically simplify the header setup.
Author
Owner

Found it for real this time I think, looks like what takes the extra time is the header-based(As opposed to meta-tag based) scanning for alt-locations, which MuWire was not adequately excluded from. I changed a bunch of stuff internally though and it's going to take time some testing in general before I'm totally comfortable releasing it, but go ahead and try the do-less branch now

Found it for real this time I think, looks like what takes the extra time is the header-based(As opposed to meta-tag based) scanning for alt-locations, which MuWire was not adequately excluded from. I changed a bunch of stuff internally though and it's going to take time some testing in general before I'm totally comfortable releasing it, but go ahead and try the do-less branch now
Author
Owner

Yeah I knew something was wrong, muwireprefpriv is currently impossible to trigger. That references the json file with the English translations of the URL bar labels for MuWire when acting on top of the built-in Private Browsing mode. It used to be possible to cause it, but Mozilla changed how extensions are used when Private Browsing is also in use so I also had to change(Private Browsing is a disposable container tab, this extension is just disposable container tabs with rules for working with I2P). I'm narrowing it down further, there are only a few more things it could possibly be. I'll ping you shortly.

Yeah I knew something was wrong, `muwireprefpriv` is currently impossible to trigger. That references the json file with the English translations of the URL bar labels for MuWire when acting on top of the built-in Private Browsing mode. It used to be possible to cause it, but Mozilla changed how extensions are used when Private Browsing is also in use so I also had to change(Private Browsing is a disposable container tab, this extension is just disposable container tabs with rules for working with I2P). I'm narrowing it down further, there are only a few more things it could possibly be. I'll ping you shortly.
Author
Owner

Hmm, maybe not. I just saw that I had a typo in the patch, should be chrome instead of chrom. When that is fixed, tree loads slowly again.

Hmm, maybe not. I just saw that I had a typo in the patch, should be `chrome` instead of `chrom`. When that is fixed, tree loads slowly again.
Author
Owner

I think it was just the lack of definition of muwireprefpriv which threw an exception and possibly failed to initialize a bunch of other things. When that symbol is defined everything works great!

I think it was just the lack of definition of `muwireprefpriv` which threw an exception and possibly failed to initialize a bunch of other things. When that symbol is defined everything works great!
Author
Owner

Thanks for checking. I can't think of a good one offhand, I've got a bunch of MuWire downloads in a plugin instance, going so I'll be able to get a much closer look on my end soon.

Thanks for checking. I can't think of a good one offhand, I've got a bunch of MuWire downloads in a plugin instance, going so I'll be able to get a much closer look on my end soon.
Author
Owner

quick fix:

--- a/background.js
+++ b/background.js
@@ -14,6 +14,7 @@ var ircpref = chrome.i18n.getMessage('ircPreface');
 var ircprefpriv = chrome.i18n.getMessage('ircPrefacePrivate');
 var extensionpref = chrome.i18n.getMessage('extensionPreface');
 var muwirepref = chrome.i18n.getMessage('muwirePreface');
+var muwireprefpriv = chrom.i18n.getMessage('muwirePrefacePrivate');
 var botepref = chrome.i18n.getMessage('botePreface');
 
 function onContextsGot(contexts) {
quick fix: ``` --- a/background.js +++ b/background.js @@ -14,6 +14,7 @@ var ircpref = chrome.i18n.getMessage('ircPreface'); var ircprefpriv = chrome.i18n.getMessage('ircPrefacePrivate'); var extensionpref = chrome.i18n.getMessage('extensionPreface'); var muwirepref = chrome.i18n.getMessage('muwirePreface'); +var muwireprefpriv = chrom.i18n.getMessage('muwirePrefacePrivate'); var botepref = chrome.i18n.getMessage('botePreface'); function onContextsGot(contexts) { ```
Author
Owner

I built from the branch, commit 1946669b82 but the shared files tree still loads slowly. Anything else I can do to help debug?

I built from the branch, commit 1946669b822db9c977a2bf0a27a81dfc563adea0 but the shared files tree still loads slowly. Anything else I can do to help debug?
Author
Owner

I think I have the MuWire issue solved in the do-less branch, but I'm taking this as a wake-up call to the performance issues I may have created by checking on tab status too often. I'll need a few days to test it, my testing documentation is pretty meager at this point but if you go to about:debugging#/runtime/this-firefox and load temporary addon you should see improvements now that the extension only moves your tabs from firefox-default --> firefox-container-X and not from firefox-container-X --> firefox-container-Y, so once you're already in the MuWire tab, the most complex checks are no longer performed. I probably have several other places where I can make it more efficient, I'll probably be ready to cut the .90 release on Friday or Saturday.

I think I have the MuWire issue solved in the do-less branch, but I'm taking this as a wake-up call to the performance issues I may have created by checking on tab status too often. I'll need a few days to test it, my testing documentation is pretty meager at this point but if you go to [about:debugging#/runtime/this-firefox](about:debugging#/runtime/this-firefox) and `load temporary addon` you should see improvements now that the extension *only* moves your tabs from `firefox-default --> firefox-container-X` and not from `firefox-container-X --> firefox-container-Y`, so once you're already in the MuWire tab, the most complex checks are no longer performed. I probably have several other places where I can make it more efficient, I'll probably be ready to cut the .90 release on Friday or Saturday.
Author
Owner

There is at least one place where the checks are configured to be blocking when they no longer probably no longer have to be, and I'm sure there are places where I could make the process more efficient in scrub.js, where the bulk of the work is done. There may be one or two remaining unneccesary "for" loops in but I've been getting rid of those as fast as I can and for now, things that trigger those loops should be fairly rare, as the remaining few happen in the course communication between the content scripts and the background scripts.

The biggest part of the problem is probably simply that it performs certain checks far too often. I have not spent much time limiting excessive checks of any type. It may also make sense to attach certain checks it now performs on every request to different types of browser events than they are attached to now in order to leverage webNavigation filters. That should let me two things, 1) use Mozilla's filtering tools do parts of the checking I do in the extension at "native speed" rather than extension speed 2) use webNavigation filters as a sort of "pre-check" for things that have 0% chance of needing further checks and bail out at the beginning of the process.

There is at least one place where the checks are configured to be blocking when they no longer probably no longer have to be, and I'm sure there are places where I could make the process more efficient in scrub.js, where the bulk of the work is done. There may be one or two remaining unneccesary "for" loops in but I've been getting rid of those as fast as I can and for now, things that trigger those loops should be fairly rare, as the remaining few happen in the course communication between the content scripts and the background scripts. The biggest part of the problem is probably simply that it performs certain checks far too often. I have not spent much time limiting excessive checks of any type. It may also make sense to attach certain checks it now performs on every request to different types of browser events than they are attached to now in order to leverage [webNavigation](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webNavigation) filters. That should let me two things, 1) use Mozilla's filtering tools do parts of the checking I do in the extension at "native speed" rather than extension speed 2) use webNavigation filters as a sort of "pre-check" for things that have 0% chance of needing further checks and bail out at the beginning of the process.
Author
Owner

Any thoughts on why the request checking and delegation themselves take so long?

Any thoughts on why the request checking and delegation themselves take so long?
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: idk/I2P_in_Private_Browsing_Mode_for_Firefox#8
No description provided.