Skip to content
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

discussion: redo 1000s cache #51

Closed
Thorin-Oakenpants opened this issue Mar 11, 2017 · 11 comments
Closed

discussion: redo 1000s cache #51

Thorin-Oakenpants opened this issue Mar 11, 2017 · 11 comments

Comments

@Thorin-Oakenpants
Copy link
Contributor

I think the 1000s cache section could do with a little love. My thoughts are the header section (or some items) needs a [SETUP] tag and information on session restores (crash recovery etc).

I gave pk some instructions the other day on how to trigger a FF crash. I'll repost them here. I need to make sure that all the session restore and resume from crash is grouped together, and to make sure it is clear what needs to be done to enable them.

@Thorin-Oakenpants
Copy link
Contributor Author

Crash!

  • Go to about:config and turn devtools.chrome.enabled to true
  • Open Scratchpad with Shift+F4
  • Switch Environment to Browser and run the following snippet
Cu.import("resource://gre/modules/ctypes.jsm");
let zero = new ctypes.intptr_t(8);
let badptr = ctypes.cast(zero, ctypes.PointerType(ctypes.int32_t));
badptr.contents;
  • Crash!

@earthlng
Copy link
Contributor

earthlng commented Mar 16, 2017

1006: they call this Fastback caching

/* 1006: disable pages being stored in memory. This is not the same as memory cache. Fastback caching

It's also not only for the paranoid but can be used to limit memory consumption.
People don't necessarily have to set it to 0 but could limit it to certain number, 10 or 20 for example.
Instead of having the default -1 which means

if this pref is negative, then we calculate the number of content viewers to cache based on the amount of available memory.

so maybe we could go with /* 1006: disable (or limit) Fastback caching

1005: could use a better explanation.
It's confusing because the title says disable storing extra session data and the option 0=all.
Mozilla uses the following comment for this pref:

// on which sites to save text data, POSTDATA and cookies
// 0 = everywhere, 1 = unencrypted sites, 2 = nowhere

maybe something like this:

/* 1005: disable storing extra session data
 * extra session data contains contents of forms, scrollbar positions, cookies and POST data
 * define on which sites to save extra session data:
 * 0 = everywhere, 1 = unencrypted sites, 2 = nowhere ***/
user_pref("browser.sessionstore.privacy_level", 2);

@earthlng
Copy link
Contributor

Oh, okay, I assumed it meant pages as in sites. But if it's memory pages or something like that, then we should not encourage changing it to anything other than 0 or the default -1

@earthlng
Copy link
Contributor

earthlng commented Mar 16, 2017

Looks like it's "sites" after all and the table on the Zine is still accurate:
https://dxr.mozilla.org/mozilla-central/source/docshell/shistory/nsSHistory.cpp#263

For the sake of completeness, this option is listed for the truly paranoid.
We should drop that line or extend it to also mention usefulness to decrease memory consumption.

or any other positive integer is also not ideal. for other values see [1] for example.

@earthlng
Copy link
Contributor

earthlng commented Mar 16, 2017

nice! a few nits:

  • disabling memory cache - I don't see how that would help the really paranoid, because they would stand out really badly. If anything, it's maybe useful for really old and shitty computers with not much RAM.
    => why do you think this is useful for the truly paranoid?
  • browser.sessionstore.interval - for the sake of SSD's we should probably enable this with a reasonably high value. (I have 60000, but even 30sec is much better than the default 15sec)
    https://www.servethehome.com/firefox-is-eating-your-ssd-here-is-how-to-fix-it/

@earthlng
Copy link
Contributor

earthlng commented Mar 16, 2017

re: SSD problem
https://bugzilla.mozilla.org/show_bug.cgi?id=1304389
they're working on it. AND with our other settings and bit of smart behaviour (like open links in new tabs and don't keep 10 history on each tab + use PB, etc) my recovery.js at the moment is only 55kb. They talk about files with 1MB and more, so for us it's not that much of a problem.
And my profile never touches my SSD anyway, but that's not the point here.

@earthlng
Copy link
Contributor

latest generation SSDs will far outlive their useful lives even with heavy use

my Vertex2 only lasted ~5 years. And I did everything possible to limit the writes. Ok, it wasn't exactly "latest generation" but I doubt my current one will last much longer.

Idk if we really need a description for cache - it's pretty self-explanatory IMO
I definitely don't like the comment out and and reset this entire section because for example 1004 can and probably should remain disabled, always. But yeah, i don't really care and I'm certainly not going to start another arguing-war about it

disabling memory cache - oh yeah, cold-boot-attack. Ok you win :)

@earthlng
Copy link
Contributor

earthlng commented Mar 17, 2017

It makes it sound way worse than it really is. "forensic data" "all over your OS" and then listing literally all the things to turn back on, with a special note about cache.
I mean we're not dealing with idiots or amateurs here. Everyone will know what Cache is and does, and whether they want it enabled or not. We give them the options, let them choose for themselves.
We don't need to over-explain everything IMO, and by doing so potentially dig ourselves a hole if something is incorrect.
/*** 1000: CACHE [SETUP] ***/ => done. but

i don't really care and I'm certainly not going to start another arguing-war about it

just my 2cents

@earthlng
Copy link
Contributor

earthlng commented Mar 17, 2017

Man! are you not reading my stuff? ;)
Let me try this one more time and you can do with it what you want:

/* 1005: disable Fastback caching
 * To improve performance when pressing back/forward Firefox stores visited pages
 * so they don't have to be re-parsed. This is not the same as memory cache.
 * 0=none, -1=auto (that's minus 1), or for other values see [1]
 * [1] http://kb.mozillazine.org/Browser.sessionhistory.max_total_viewers ***/
   // user_pref("browser.sessionhistory.max_total_viewers", 0);
  • page does not refer to a web page here - is incorrect
  • (which is old and not maintained) - but still accurate! I linked to the table in DXR

=> shorter, sexier and accurate

ps. the rest is great! I haven't looked at the icons-stuff, so atm I can't really help you there.

@earthlng
Copy link
Contributor

man! you gotta be shitting me??!! ANOTHER full patch to review? we should really use PR for this kind of thing.

@earthlng
Copy link
Contributor

earthlng commented Mar 20, 2017

browser.chrome.favicons - see http://kb.mozillazine.org/Browser.chrome.favicons

  • browser.chrome.site_icons must be true for this preference to have an effect.
  • Conversely, browser.chrome.site_icons should be false when this preference is false to disable site icons and favicons completely.

IMO we should put them both under 1031. And we can enable 1030 by default IMO.

edit: for relevance 1031 should probably be 1030.

edit2: other than that 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants