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

GM 3.5beta1+: GM_getResourceText seems slow #2346

Closed
janekptacijarabaci opened this issue Jan 1, 2016 · 27 comments
Closed

GM 3.5beta1+: GM_getResourceText seems slow #2346

janekptacijarabaci opened this issue Jan 1, 2016 · 27 comments
Milestone

Comments

@janekptacijarabaci
Copy link
Contributor

Ad https://groups.google.com/forum/#!topic/greasemonkey-users/o504w12bu6Y

Confirmed on:
GM 3.5beta1+

Not confirmed on:
GM 3.4.1-

See 6fe96d6#diff-4b4ab9adda07ca484ef0efbd20401182L28

@the8472
What is the best solution to fix this problem?

@the8472
Copy link
Contributor

the8472 commented Jan 1, 2016

It would probably be more efficient to load the style from the resource URL instead of adding it as text.

GM_addStyle does nothing more than just injecting a <style> tag into <head>. I suggest doing it manually and setting the src of the style to the resource URL.

@janekptacijarabaci
Copy link
Contributor Author

Ad GM_addStyle: However, it has nothing to do with it...

After returning lines:
6fe96d6#diff-9f06c50d889eb78e6776c4b4fc7314cbL36
6fe96d6#diff-4b4ab9adda07ca484ef0efbd20401182L28
the differences are orders of magnitude... (fractions of milliseconds vs tens of milliseconds)

The file loads slowly - any file...

@janekptacijarabaci
Copy link
Contributor Author

AFAIK: This line does not make sense (this condition will never be fulfilled):
6fe96d6#diff-4b4ab9adda07ca484ef0efbd20401182R30

...if nothing changes, e.g.:
(https://github.com/greasemonkey/greasemonkey/blob/3.6/modules/miscapis.js#L31)

From:

return GM_util.fileXhr(dep.file_url, "text/plain");

To:

dep.textContent = GM_util.fileXhr(dep.file_url, "text/plain");
return dep.textContent;

But this does not solve the original problem... (only after the second load)

@arantius arantius added this to the 3.7 milestone Jan 1, 2016
@the8472
Copy link
Contributor

the8472 commented Jan 1, 2016

@janekptacijarabaci

That's intentional to reduce memory footprint. resource files can be very large, reading them into memory and sending them to the child process would be a huge waste if they are not used in text form or only loaded via URL.

Basically, the encouraged way is to load them via URL, not via text string.

Ad GM_addStyle: However, it has nothing to do with it...

It does. The problem with it is that it takes a string and not a URL. Using the URL directly should be faster. With a URL FF can pass down the file contents efficiently (shared memory) instead of copying javascript strings.

AFAIK: This line does not make sense (this condition will never be fulfilled):

Yeah, it was kinda meant as backwards compatibility so a parent process Script object could be passed to getResourceText, but that never happens.

@janekptacijarabaci
Copy link
Contributor Author

I understand why it was done. But... Allocation of memory ≠ memory leaks. Accessing data from disk is slow (in this case, no cache is made).
But what is better? So maybe let the user decide in the options?!
getResourceText: this is useful sometimes...

@the8472
Copy link
Contributor

the8472 commented Jan 2, 2016

I think the user can simply adjust his script by using GM_getResourceURL instead of GM_getResourceText.

@bombledmonk
Copy link

Hi, I wrote the email that initiated this issue.
I guess I'm a little confused on what the intended use of @resource is. I was under the impression that resources were to be used so the browser doesn't have to keep hitting third party websites for info. I was using it to avoid having to use a formal host for my css. I'm using dropbox so I didn't really want to have a request going that direction on every single page load for every single resource. My users load 100k+ pages a month and that was my solution to avoid perceived ire of the dropbox overlords.
I suppose I'm just using it outside of intended use case though it seems like a reasonable use to me.

That still begs the question, no matter what the resource is a 100ms+ access time seems like a huge penalty to use the @resource feature. Is this really the intended operation of the @resource?

@the8472
Copy link
Contributor

the8472 commented Jan 2, 2016

If this is a response to my suggestion of using GM_getResourceURL, have you read its documentation?

http://wiki.greasespot.net/GM_getResourceURL

@bombledmonk
Copy link

I read the documentation, but it wasn't clear to me as to how it would help or exactly what it did. This was partially because there was a bunch of conflicting explanations on the web referencing older greasemonkey versions. After I understood a little more about how it worked, I altered the script to use GM_getResourceURL and it mostly alleviated the issue in Firefox. The css files loaded quickly, but the resulting web page wasn't identical. That part is probably the result of some css precedence conflicts that I'll have to track down myself.

I know it's not the place to bring up problems with Tampermonkey, but unfortunately (for me) this solution isn't cross platform compatible.(unless I'm doing something wrong, which is quite likely)

for firefox I did this:

    for ( var x in cssNames){
        var thetext = GM_getResourceURL(cssNames[x]);
        $('body').prepend('<link rel="stylesheet" href="'+thetext+'">')
    }

for tampermonkey I did this and it worked:

    for ( var x in cssNames){
        var thetext = GM_getResourceURL(cssNames[x]);
        $('body').prepend('<link rel="stylesheet" href="data:text/css;base64,'+thetext+'">')
    }

After I looked at the tampermonkey solution a bit further it doesn't even make sense given the full encoded text ends up in the href, but it does work for some reason.

Is there something simple I'm missing that would make it cross platform?

That still leaves a dangling question. What is the intended use of GM_getResourceText with access times being so slow? Is the GM_getResourceURL meant to replace this most of the time?

@the8472
Copy link
Contributor

the8472 commented Jan 4, 2016

$('body').prepend

addStyle puts it in the <head>, not the <body>

What is the intended use of GM_getResourceText with access times being so slow?

For those times where you really want manipulate the string instead of just putting it somewhere in the DOM.

The sad state of things is that the API is synchronous and providing potentially very large chunks of data in a synchronous manner either eats memory or is slow.

Maybe we should deprecate it and instead allow GM_xmlhttprequest to fetch URIs provided by GM_getResourceURL. That way it could be made asynchronous.

Is the GM_getResourceURL meant to replace this most of the time?

whenever possible, yes.

I know it's not the place to bring up problems with Tampermonkey,

well, what does TM do?

@bombledmonk
Copy link

Tampermonkey exhibits the old behavior of GM_getResourceURL. It gives you the full base64 representation of the the css file, not just a local url.

@the8472
Copy link
Contributor

the8472 commented Jan 4, 2016

Then it shouldn't really need special treatment. TM will load it as data URI, GM will load it through its custom uri scheme. Maybe you have to explicitly specify the type attribute for data uris?

@derjanb
Copy link

derjanb commented Jan 4, 2016

TM returns a base64 encoded string instead of a real dataURI here (see Tampermonkey/tampermonkey#254). This issue will be fixed soon. Sorry for any inconvenience.

@arantius
Copy link
Collaborator

This bug is either not valid, or is only valid in some specific context and needs more detail.

I wrote a test script: https://gist.github.com/arantius/2e385ac853260c4e6612

It takes an arbitrary @resource and calls GM_getResourceText() 500 times. The data it logs is:

before 2016-01-20T20:18:30.584Z
after 2016-01-20T20:18:31.265Z
total time 681 ms
mean per call 1.362 ms

It takes ~1.5ms, not 100ms, to call GM_getResourceText(). On my machine at least, running:

Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0

If you disagree, please run the exact script linked, and paste both the log and the "User Agent" line from the about:support page.

@arantius arantius modified the milestones: 3.8, 3.7 Jan 20, 2016
@janekptacijarabaci
Copy link
Contributor Author

Ups, I'm sorry.

@bombledmonk
Copy link

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0

I ran the linked test script and came up with better times for large N

before 2016-01-20T23:13:43.805Z
after 2016-01-20T23:13:46.178Z
total time 2373 ms
mean per call 4.746 ms

but with N=1

before 2016-01-20T23:17:46.523Z
after 2016-01-20T23:17:46.768Z
total time 245 ms
mean per call 245 ms

This total time ranged from 190ms ~ 460ms across many separate refreshes of a test page where N=1.

The best solution thus far solution to my original problem is likely the slightly less intuitive GM_getResourceURL(). Hopefully this can be a cross platform answer if/when @derjanb releases the next stable version of Tampermonkey. It's hard to tell with Tampermonkey as the bugs are tracked on github, but it's otherwise not open source aside from what someone can glean from minified javascript.

@arantius
Copy link
Collaborator

With N=1 I see e.g.

before 2016-01-21T14:19:49.156Z 
after 2016-01-21T14:19:49.162Z
total time 6 ms 
mean per call 6 ms 

More variance, rarely up to 13ms, but still nowhere close to 100.

Your user agent shows Windows. Maybe Windows is doing something unusual here. I'll have to remember to try to test this when I'm near a Windows box to help confirm. Only have Linux and Mac nearby at the moment.

@bombledmonk
Copy link

Minor update. I've confirmed the slow behavior on two more Win7 installs.

@janekptacijarabaci
Copy link
Contributor Author

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0

Ad https://gist.github.com/arantius/2e385ac853260c4e6612:

before 2016-01-21T14:41:43.884Z
after 2016-01-21T14:41:44.666Z
total time 782 ms
mean per call 1.564 ms

With N=1:
before 2016-01-21T14:43:59.019Z
after 2016-01-21T14:43:59.656Z
total time 637 ms
mean per call 1.274 ms

With N=2:
before 2016-01-21T14:44:05.371Z
after 2016-01-21T14:44:06.002Z
total time 631 ms
mean per call 1.262 ms

@bombledmonk
Copy link

@janekptacijarabaci are you sure that's correct? With N=1 there's no way for the mean time per call to be 1.274ms when the total time is 637ms

With N=1:
before 2016-01-21T14:43:59.019Z
after 2016-01-21T14:43:59.656Z
total time 637 ms
mean per call 1.274 ms

With N=2:
before 2016-01-21T14:44:05.371Z
after 2016-01-21T14:44:06.002Z
total time 631 ms
mean per call 1.262 ms

@janekptacijarabaci
Copy link
Contributor Author

I'm sorry.

N = 500

"N=1" = running second time

"N=2" = running third time


N=1:

Running first time
before 2016-01-21T15:40:38.390Z
after 2016-01-21T15:40:38.391Z
total time 1 ms
mean per call 1 ms

Running second time
before 2016-01-21T15:40:49.470Z
after 2016-01-21T15:40:49.471Z
total time 1 ms
mean per call 1 ms

@CoolCmd
Copy link

CoolCmd commented Feb 16, 2016

janekptacijarabaci, whats your CPU? try to uninstall antivirus.

all tests with N=1:

intel sandy bridge, 1.6GHz, 1 core, win7 x64:

12:02:19.300 before 2016-02-16T09:02:19.300Z GM_getResourceText_Test.user.js:13:1
12:02:19.371 after 2016-02-16T09:02:19.371Z GM_getResourceText_Test.user.js:18:1
12:02:19.372 total time 71 ms GM_getResourceText_Test.user.js:19:1
12:02:19.373 mean per call 71 ms GM_getResourceText_Test.user.js:20:1

intel sandy bridge, 3.4GHz, 4 cores, win7 x64:

12:04:02.645 before 2016-02-16T09:04:02.645Z GM_getResourceText_Test.user.js:13:1
12:04:02.647 after 2016-02-16T09:04:02.647Z GM_getResourceText_Test.user.js:18:1
12:04:02.647 total time 2 ms GM_getResourceText_Test.user.js:19:1
12:04:02.647 mean per call 2 ms GM_getResourceText_Test.user.js:20:1

intel sandy bridge, 1.6GHz, 4 cores, win7 x64:

9ms .. 37ms

may be with intel atom 100ms is "normal" for FF44 & GM 3.6? or with stupid antivirus.
GM_getResourceText surprisingly CPU-intensive and core-dependent. poor optimization i think...
and see 3rd test. thread sync problem?

@arantius
Copy link
Collaborator

In a post-e10s world, stuck with this old synchronous API, in order to service a GM_getValue() call, we have to: send a synchronous message from child to parent process, receive that message, create the appropriate object *, call its get method, open the DB file, issue the query, return that value (thus implicitly doing a serialization call so that it can be), passed via message reply back to the child process, where it's deserialized, then for type safety we have to deserialize again, then we can finally return the value.

There's a lot of things going on here. On any one machine any one of those steps could be slow for any number of reasons.

* Perhaps memoizing this could help?


I've updated the measurement script to v2. Now I get e.g.

Running GM_getResourceText test; NUM = 250
Over 250 samples 535.75 total time.
min = 1.1549999999999727 max = 59.09500000000003 mean = 2.0931200000000003 median = 1.5049999999999955 std = 4.070640579134692

Which is still pretty reasonable, though I do see the occasional slowish (~60ms above) call. By manually inspecting the raw samples, I can see that (for me) the worst case is about NUM=7 which gives:

Over 7 samples 339.88 total time.
min = 1.2549999999999955 max = 192.32 mean = 48.50714285714286 median = 14.754999999999995 std = 67.31983622090537

Because the slow calls all seem to be bunched at the front; after ~10 they're all ~1ms each.

A mean of 50ms is indeed "slow". This was an outlier though, usually I'm max/mean in the 60/15 range. Accounting for where that time goes will not be easy, but is something to pursue.

@the8472
Copy link
Contributor

the8472 commented Feb 19, 2016

open the DB file

are you sure you have the right bug? this is about resources, not get/set value

@arantius
Copy link
Collaborator

Oh foolish me! Above I was describing GM_getValue(), not GM_getResourceText().

For the latter we're doing the fileXhr thing, which all works in the child process. (For now.) And I have even less idea what to do about being faster.

@the8472
Copy link
Contributor

the8472 commented Feb 19, 2016

I think the only real solution is to offer an async API. It would not make individual calls faster, but it would allow multiple of them to be performed / awaited in parallel.

Maybe just add an argument and if it is present then return a promise instead of the string? Consuming code can be backwards-compatible by type-checking the returned object and wrapping it in a promise if it's a string and then use the same code path.

@CoolCmd
Copy link

CoolCmd commented Feb 27, 2016

I think the only real solution is to offer an async API. It would not make individual calls faster, but it would allow multiple of them to be performed / awaited in parallel.

WebExtensions will be available soon. They have async API and they are easy to create.

@arantius arantius modified the milestones: Pony, 3.8 Mar 4, 2016
@arantius arantius modified the milestones: Pony, Bankruptcy Jul 25, 2017
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

6 participants