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

Move GM_setValue storage #1798

Closed
arantius opened this issue Sep 4, 2013 · 12 comments
Closed

Move GM_setValue storage #1798

arantius opened this issue Sep 4, 2013 · 12 comments
Milestone

Comments

@arantius
Copy link
Collaborator

arantius commented Sep 4, 2013

https://bugzilla.mozilla.org/show_bug.cgi?id=872981

https://blog.mozilla.org/addons/2013/09/03/compatibility-for-firefox-24/

We haven’t been explicit enough about this, but the preferences system is not meant for general data storage. The crashes caused by certain extensions pushed us to enforce a limit of 1Mb per preference, and we will lower it to 4Kb soon. For now, this only introduces a warning if you pass that limit.

There's probably some scripts setting big values (if big is >4KB). If we get ahead of this they might not break.

@janekptacijarabaci
Copy link
Contributor

#1768 ?

I have a size of a few MB...

Otherwise:

From:

var valueGM = (typeof GM_setValue === "function"); // etc.
var valueLS = ("localStorage" in window && window.localStorage != null);

To:

// var valueGM = (typeof GM_setValue === "function"); // etc.
var valueGM = false;
var valueLS = ("localStorage" in window && window.localStorage != null);

Or:

From:

// @grant GM_setValue

etc.

To:

about:config dom.storage.default_quota

But the text is the text (prefs.js) :-/

@arantius
Copy link
Collaborator Author

arantius commented Oct 2, 2013

Wherever this gets moved to, it would be really nice to have the storage more structured; i.e. the script ID and the value name stored separately.

arantius added a commit that referenced this issue Oct 2, 2013
@arantius
Copy link
Collaborator Author

arantius commented Oct 8, 2013

Values are not properly migrated upon script update/reinstall.

@arantius arantius reopened this Oct 8, 2013
arantius added a commit to arantius/greasemonkey that referenced this issue Oct 10, 2013
Since the values are now stored in a file alongside the script, there's no practical place to keep them on uninstall anymore.  I.E. Always remove them on uninstall.

Refs greasemonkey#1798
arantius added a commit to arantius/greasemonkey that referenced this issue Oct 10, 2013
Except in one legacy migration codepath.  Values are now stored in per-script DBs instead.

Refs greasemonkey#1798
arantius added a commit to arantius/greasemonkey that referenced this issue Oct 10, 2013
arantius added a commit to arantius/greasemonkey that referenced this issue Oct 10, 2013
@arantius
Copy link
Collaborator Author

Reopening to confirm working behavior on Windows (locking file systems .....!!!!!).

@arantius arantius reopened this Oct 22, 2013
arantius added a commit to arantius/greasemonkey that referenced this issue Oct 23, 2013
@arantius arantius reopened this Oct 23, 2013
@arantius
Copy link
Collaborator Author

I don't know why yet but I'm rarely seeing:

System JS : ERROR resource://greasemonkey/miscapis.js:78
                 NS_ERROR_STORAGE_IOERR: Component returned failure code: 0x80630002 (NS_ERROR_STORAGE_IOERR) [mozIStorageStatement.execute]

Errors logged. Never while I'm doing things, but lots when I'm idle for a while and come back.

@visua0
Copy link

visua0 commented Oct 31, 2013

Can anything be done about the freezes that occur when writing to values.db?
Take for example this script https://ihavenoface.me/stable-v3/4chan-X.user.js
It calls setValue when you scroll down unread comments. Using 1.12 there is no issue no matter how fast you scroll down, but using the latest beta even scrolling down a single comment makes Firefox stutter.
Calling setValue now causes 16 write events, so I'm not surprised.

@arantius
Copy link
Collaborator Author

arantius commented Nov 1, 2013

First: thanks very much for your feedback about beta releases! It's hard for me to accurately simulate/expect real user behavior.

In an attempt to get some objective data I threw this script together:
https://gist.github.com/arantius/7267388

It does a benchmark of get/set calls. With GM 1.12, it outputs:

[11:46:48.082] "Starting set benchmark ..."
[11:46:49.273] "10,000 set calls took" 1191
[11:46:49.273] "Starting get benchmark ..."
[11:46:50.263] "10,000 get calls took" 990
[11:46:50.263] "Starting get/set benchmark ..."
[11:46:52.445] "10,000 get/set calls took" 2182

Then I tried with the latest 1.13 beta (5 at this time). I got the fifth or sixth unresponsive script popup and gave up. I had to drop the iterations way down to avoid them, and then:

[12:01:04.942] "Starting set benchmark ..."
[12:01:08.560] 40 "set calls took" 3617
[12:01:08.560] "Starting get benchmark ..."
[12:01:08.579] 40 "get calls took" 19
[12:01:08.579] "Starting get/set benchmark ..."
[12:01:13.784] 40 "get/set calls took" 5205

That means that get operations went from 990 / 10000 = 0.10 ms to 19 / 40 = .48 ms or take almost 5x as long. But set went from 1191 / 10000 = 0.12 ms to 3617 / 40 = 90.43 ms or over 900x slower. That indeed is quite the hit.

If you've got spare time, could you check what sort of results you get?

@arantius
Copy link
Collaborator Author

arantius commented Nov 1, 2013

Setting PRAGMA journal_mode = WAL; gives:

[12:11:50.711] "Starting set benchmark ..."
[12:11:52.662] 500 "set calls took" 1950
[12:11:52.662] "Starting get benchmark ..."
[12:11:52.760] 500 "get calls took" 97
[12:11:52.760] "Starting get/set benchmark ..."
[12:11:55.005] 500 "get/set calls took" 2244

Which makes set 3.9ms (under 5% of the run time). Setting PRAGMA journal_mode = MEMORY; gives:

[12:13:47.169] "Starting set benchmark ..."
[12:13:48.997] 40 "set calls took" 1828
[12:13:48.997] "Starting get benchmark ..."
[12:13:49.014] 40 "get calls took" 17
[12:13:49.014] "Starting get/set benchmark ..."
[12:13:51.785] 40 "get/set calls took" 2771

Worse than WAL!? Hardly any better than we started with. I had tried WAL briefly early on, but it seemed to produce giant DB files. That might have been a poor/snap judgment.

arantius added a commit to arantius/greasemonkey that referenced this issue Nov 1, 2013
@visua0
Copy link

visua0 commented Nov 2, 2013

Thanks for actually looking into it.
Running your script gave me similar results.

1.12:

07:50:37.982 "Starting set benchmark ..."
07:50:38.777 "10000 set calls took 0.0795 each"
07:50:38.777 "Starting get benchmark ..."
07:50:39.369 "10000 get calls took 0.0592 each"
07:50:39.369 "Starting get/set benchmark ..."
07:50:40.776 "10000 get/set calls took 0.1407 each"

1.13b5:

07:52:20.071 "Starting set benchmark ..."
07:52:32.044 "100 set calls took 119.73 each"
07:52:32.044 "Starting get benchmark ..."
07:52:32.059 "100 get calls took 0.15 each"
07:52:32.059 "Starting get/set benchmark ..."
07:52:45.458 "100 get/set calls took 133.99 each"

In my case writing was 1500x slower.

I can't build the xpi myself so I can only hope that WAL mode is fast enough to get rid of the problem.

@arantius
Copy link
Collaborator Author

Version 1.13beta6 has been uploaded. Let me know if this performs any better, and if so, if that's good enough.

@visua0
Copy link

visua0 commented Nov 13, 2013

It's working great. No freezes during scrolling at all now. Thanks for the fix.
I could imagine it still causing some freezes with a slow HDD, but caching setValues or something might be more trouble than it's worth.

@basilevs
Copy link

basilevs commented Jan 7, 2014

Unused constant: 47bc190#commitcomment-5003232

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

No branches or pull requests

4 participants