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

Cache the CSS contents (in jQuery mode) #335

Closed
mossroy opened this issue Jan 15, 2018 · 7 comments
Closed

Cache the CSS contents (in jQuery mode) #335

mossroy opened this issue Jan 15, 2018 · 7 comments
Assignees
Milestone

Comments

@mossroy
Copy link
Contributor

mossroy commented Jan 15, 2018

Following the discussions in #130, this is about putting the CSS contents of a ZIM file in a cache.
@Jaifroid suggests to port/adapt the work of @sharun-s in kiwix-js, as it seems to be a very efficient optimization.

@mossroy mossroy added this to the v2.3 milestone Jan 15, 2018
@Jaifroid
Copy link
Member

Thanks, @mossroy. The suggested code is in https://github.com/kiwix/kiwix-js/tree/Backport-CSSCache .

In testing, it would be a good idea to use one of the "modern" ZIMs, I suggest wikipedia_en_medicine_novid_2018-01.zim. The reason for using this or similar to test is that the new mobile-oriented ZIMs require a lot of stylesheets. Other than the front page, many pages require between four and seven stylesheets to be decompressed. Some of them are dummy stylesheets containing nothing, but they're still costly to decompress. Hence you see a dramatic speed improvement in Kiwix JS once the app has had a chance to cache most or all of the required CSS. Watch the Console to see what it's doing.

@mossroy
Copy link
Contributor Author

mossroy commented Jan 15, 2018

I had a look at the code and tested on PC with Ray Charles archive (I could not test on my FirefoxOS device, and with the ZIM file you suggests, but will do later).
The place to remove the cache content is certainly right for the FirefoxOS DeviceStorage way to read ZIM files.

I did not measure the performance, but it gives a real visual improvement : as the CSS is instantly loaded, the content "moves" much less after displaying the text.
This optimization is (unfortunately) specific to the jQuery mode. But it does not seem to add a lot of complexity.

As there should not be a lot of CSS content in a ZIM file, I think it's not really necessary to put a size limit for this cache.

Regarding the code itself, some variable/function naming look questionable, like newCC. But we can discuss that in the PR.

I'll try to test more later.

@Jaifroid
Copy link
Member

OK, thanks @mossroy. Just to be clear, I haven't put in any code to clear the cache anywhere except in setLocalArchiveFromFileList. And that is definitely not hit on changing archives using the FFOS Device Storage menu. So I leave it to you to insert that code at the best spot for FFOS users. I don't know if there's a "universal" spot we could find to insert the cache-clearing code only once?

Feel free also to change any of the variable names to anything more consistent. Then I can do a PR.

@mossroy
Copy link
Contributor Author

mossroy commented Jan 15, 2018

I've refactored the code a bit and pushed on your branch. It's open to comments.
I've added the code to clear the cache on FFOS (you were right, I spoke too soon) : it's working fine on my devices.
For some reason, the wikipedia_en_medicine_novid_2018-01.zim seems to require a lot of memory to open any article (on this branch as on master : it does not seem to come from the cache). On the ZTE Open C (512MB of RAM), it crashes the app (out of memory). But on the Flame (1GB of RAM), it works fine.

The CSS cache indeed gives a huge speed gain!

@Jaifroid
Copy link
Member

Great, that looks good! I'll do a PR.

Memory issues: I believe this is probably the same thing that crashes xzdec.js on loading pages like Sine.html from Wikipedia. In this case it's not a mass of SVGs being sent to the decompressor, it's up to seven CSS files all trying to decompress at the same time. It causes a logjam, which is noticeable in a delay in the browser for opening the first page of the latest WikiMed ZIM. If you watch the console you'll see it takes an inordinate amount of time to decompress the CSS on the startup page -- this is somewhat masked in Firefox on the desktop because it seems to have better pipeline management, but it's pretty noticeable in Edge on the desktop, and in Firefox on an old / low memory device. Hence the huge speedup of caching.

Solutions:

  1. Send the CSS one-by-one to the decompressor (or send no more than three at a time). This is what I was forced to do with SVGs, because sending 20 at a time crashes all the browsers even on the most powerful systems. It requires some code refactoring and active management of the pipe, as you have to prevent programme execution from continuing (effectively end the programme) until you've received the CSS back from the xzdec promise.

  2. Add the common CSS used in the mobile-style Wikipedia-derived ZIMs into the file system. Especially the completely empty ones, which are being decompressed for no reason at all (large overhead just setting up the decompressor environment, only to decompress a comment saying "dummy file" or somesuch).

@mossroy
Copy link
Contributor Author

mossroy commented Jan 16, 2018

I understand. You did a very good job with @sharun-s at diagnosing this, and implementing a working workaround.
I'd like to avoid handling such pipe in kiwix-js (because it's probably a lot of code, and specific to the jQuery mode) and/or storing some ZIM-dependant content in the code.
I'm hoping #219 or #116 might solve this issue.

@Jaifroid
Copy link
Member

Implemented with commit 8445238.

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

No branches or pull requests

2 participants