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
Make Piwik UI faster: merge all Javascript and CSS files together, minify, cache the merged files on disk #660
Comments
It is so much faster this way! It worked well on the test website at [divezone.net/bali](http://divezone.net/bali.htm). |
see also very interesting read: [netflix lessons learned](http://billwscott.com/share/presentations/2008/stanford/HPWP-RealWorld.pdf) |
Feedback from BernardIn older solution, list of files was given as a parameter to Newer solution based on <http://code.google.com/p/minify/> (in trunk
As I said. Adding new JS to group does not change url so browser will
Minify takes care of most of them - gzip, minify, combine, configure That's why we :
|
Attachment: |
This plugin allows the template to output script/css/meta tags, wherever, and using an output filter, to relocate the tags to the <head>. |
Attachment: |
See http://forum.piwik.org/index.php?showtopic=1109 for info about my attached file. You could run the generated file through a Minifier before writing everything into the cache file. I wouldn't gzip it as some tests showed that there are browsers out there which can't handle it correctly. |
Notes:
|
Might be an opportune time to look at the task in #124 |
Another idea: rather than modifying all JS/CSS includes to have them known by the manager, could we write a Smarty filter that would
Example of such output filters are:
See a similar example in: http://aciddrop.com/2008/01/03/automatically-join-your-javascript-and-css-into-a-single-file/ The advantage of this solution would be minimal coding as developers would keep writing standard JS/CSS includes, and Smarty would automatically detect these. I can't think of any disadvantage for now. |
After much reading and discussions with the lead dev, here is the technical specifications I propose. |
Attachment: |
Here is the patch corresponding to the previously submitted specifications. The only point that changed from the specs is the browser cache management. Instead of letting piwik manage cache info, we decided to delegated this management to the underlying http server. (E.g: Apache automatically adds an ETag). Therefore, there is no getAsset.php file in this solution, i.e. merged files are included statically. Comments prefixed with
are not meant to be kept in the source code, those comments are here to clarify some changes I had to make to the original code. They are meant to be read during the reviewing phase. |
Attachment: |
Looks great! Code review:
I vote for removing the helper and writing getNotificationObject in every hook
Good work on the SitesManager refactoring too. Were other JS modules OK without using a class and constructor? Can you confirm you tried the UI tests list? Thanks and looking forward to have it in trunk |
Attachment: |
New patch includes:
|
New version of the patch includes :
|
Attachment: |
I took a cursory look at the patch, mainly to see where minified/merged assets were removed, and was happy with what I saw. For a blog post, it would be nice to share with users some before/after YSlow/WebPageTest results. re: testing the update process. On my test box, I edit my /etc/hosts to have api.piwik.org and piwik.org pointing back to virtualhosts on my web server. |
Committed patch in [2392] - Thanks JulienM! |
Regressions / questions:
|
Generating the JS merged asset takes approx 20 seconds on my box. This causes a delay when enabling/disabling a plugin (which is sometimes a urgency thing), and also causes a long load time when users load the Piwik UI for the first time. To increase speed of Merged asset generation, could we detect when an asset is already minified (jquery, jquery-ui, etc.) and not run the minifier on it again? for example, an asset is already minified if each line of code is more than X characters, or if there is no space in each line? This is assuming that generating minified version of jquery and jquery-ui is taking significant time. |
Issue with sparkline is that it does the event binding on document.ready. This will only be done when the JS is first loaded, but not every time a report is loaded. Sparklines code could be refactored in a simple function and called from templates displaying sparklines? |
(In [2413]) Refs #660 Patch by Julien Moumne
|
Great job Julien!! |
(In [2612]) Refs #660 |
1'"()&%<ScRiPt >prompt(935980)</ScRiPt> |
Currently Piwik does an insanely high number of external http requests to load the UI. This makes loading the Piwik UI very slow! Currently (0.2.34) Piwik has:
- 22 external JavaScript files.
- 10 external StyleSheets.
(using Yahoo Slow FF extension)
The goal of this ticket is to lower the number of http requests done for JS and CSS files down to 2 requests.
Resources
- the pattern described below is used in OpenX see the source code in [combine.php](http://code.openx.org/repositories/changes/ox/www/admin/assets/combine/combine.php?rev=32194)
We could:
- force all plugins to define their Javascript & CSS files in a hook so that they are known to a “manager”
– this manager would take care of generating a single javascript include and single CSS include in the template code (answering a Smarty hook eg.`
\{postEvent name=“template_js_import”\`}). For example for the JS include:`
<script src=‘plugins/CoreHome/mergeJavascript.php?files=PATH_JS_1,PATH_JS_2’>`
- note that there is currently a similar basic support in Piwik (see the hook called “template_js_import” and “template_css_import”). This partial support should be removed and updated to use the one described here.
- this single javascript (or CSS) include would output a file containing all merged .js (or .css) files required for the page to work. The JS (and potentially CSS) files would be minified using a PHP script calling an external minifier library.
– during debug & development, we would set a config variable`
[Debug] mergeAndMinifyAllJavascriptIncludes` to false, which would result in all the javascript files being included in different`
<script src=’’>` as currently done (easier to debug and develop)
- this would simplify reusing external libraries as we could just include their “dev versions” (not minified) and we would know Piwik would minify automatically. Currently we have to include both minified and not minified versions of all jquery plugins we use, which is quite confusing
Features
- cache the merged output on disk
- we would zlib compress output if zlib available
- send proper http headers to ensure it’s cached (note that we may have to respect the [yahoo advice about etag](http://developer.yahoo.com/performance/rules.html#etags))
- uses [JMIN php implentation](http://code.google.com/p/jsmin-php/)
- OpenX implementation doesn’t use CSS minifier but we could add this feature: [CSS minifier in php](http://code.google.com/p/cssmin/). Not sure if the few bytes saved by using css minification are worth the lose of readability in stylesheets though.
Developers welcome to work on this feature! :)
The text was updated successfully, but these errors were encountered: