This application is not used any more and exists only for history. Post new tickets on the Github account.
Cette application n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Ticket #860 (closed enhancement: fixed)

Opened 6 years ago

Last modified 5 years ago

JS file concat and load at the bottom

Reported by: dsdenes Owned by:
Priority: normal Milestone: Jelix 1.2 beta
Component: jelix:core response Version: 1.1
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

According to http://developer.yahoo.net/blog/archives/2007/07/high_performanc_5.html there should be a possibility to load some JS file at the bottom of the page.

The JResponseHtml::addJSLink would have an additional parameter: bottom. If the file would be loaded at the bottom, the JResponseHtml::output would place ONE js link befor </body>. This link would point to a js type response file with a hash parameter (JResponseJs). JResponseHtml::output would read the files locally and concats/minifies them into one file and names this file as the md5hash of the array of required files. JResponseJs would read the file according to the parameter.

Attachments

ticket_860.diff (25.1 KB) - added by bricet 5 years ago.
Yet another bug about jsUniqueUrlId and cssUniqueUrlId. Really, really sorry …
ticket_860_take2.diff (25.3 KB) - added by bricet 5 years ago.
New patch applying bibo's comments
ticket_860_take3.diff (25.3 KB) - added by bricet 5 years ago.
Yet yet another bug solved (small fix)

Change History

comment:1 Changed 6 years ago by laurentj

  • Owner set to laurentj
  • Priority changed from normal to low
  • Component changed from jelix to jelix:core response

JResponseHtml::output would read the files locally and concats/minifies them into one file and names this file as the md5hash of the array of required files

it is very bad for performance, even with cache (doing md5 hash etc has a cost)..

This sort of things should not be done at runtime, but with developer's tools.

The main goal of Jelix is the performance.

Load at bottom, why not, but concat : I'm not a fan.

comment:2 Changed 6 years ago by bballizlife

This could be a good idea but only if the generated file is cached. So it would be generated once.

comment:3 Changed 6 years ago by laurentj

Well, at least, we could have a jResponseHTMLcompress or something like that which could implements this things, instead of having this features directly into jResponseHtml... So we could keep a "lighter" html response (which is yet heavy).

@bballizlife : even if the generated file is cached, we have to verify the modified date of each file, then generated an md5 hash etc... Having this feature has a cost on the server side IMHO.

comment:4 Changed 6 years ago by laurentj

  • Owner laurentj deleted

comment:5 Changed 6 years ago by dsdenes

During the development the regularly server side costs would be a filemtime() at each file of the js file set (<10). The naming hash wouldn't care about the content, only the pathes.

Yes, concating and minifying cost a lot at server side, but reduces the script size by half, so greatly improves the user performance feeling. And while doing it with developer tools, the running files would be unreadable and must to do the procedure at every file change, with the automatic script they would remain redable/editable and must not care about file changings.

In the production version the generated cache files should be persistent, and have to be cleared manually at every affected file change.

comment:6 Changed 6 years ago by laurentj

  • Priority changed from low to normal

after thinking about it, it is not a bad idea after all.

comment:7 Changed 6 years ago by bballizlife

I found this blog post this week-end from Eric Daspet : http://performance.survol.fr/2009/03/encore-des-outils/

This could help with this ticket.

comment:8 Changed 5 years ago by bricet

I propose here a patch for jResponseHtml.

Patch howto

Apply the following patch on Jelix trunk, create lib/minify, download http://minify.googlecode.com/files/minify_2.1.3.zip and unzip it in lib/minify

New features introduced by the patch

The purpose of this new feature is to concatenate all JS files linked in <head> section of jResponseHtml, minify the result, store it in folder www/cache/minify/js/ and serve this cached result as a one-for-all js file (the filename is an md5 of concatenated file names - including pathes).

It is also possible to add a filemtime param to the linked file. That way, you can set your server to add very long expires http header and still being able to ensure a get request immediately after any modifications to your js.

This simple behaviour is actually more complex. There are situations when you can't/don't want to concatenate/minify a file :

  • when it is an absolute url
  • when you specifically don't want, for specific reasons
  • parameters given when including the file are different (additionnal attributes given to addJSLink)

So in those case, and for those files, concatenation/minification doesn't occur. But to preserve inclusion order, we concatenate/minify all files we can, then serve this specific file, then begin again this process until all files are served.

IE specific JS are included after other JS. They are concatenated/minified specifficaly and included with an IE magic comment (as it used to be before this patch).

The behaviour is exactly the same for CSS. IE specific CSS are not concatenated since they may have specific magic comments. But they are minified.

Every CSS content is looked up to discover path usage and replace each token relatively to the minify cache result.

To use those features, this patch adds the following new section and options in defaultconfig.ini.php :

[jResponseHtml]
;concatenate and minify CSS and/or JS files :
minifyCSS = off # [on|off] : concatenate/minify CSS files
minifyJS = off # [on|off] : concatenate/minify JS files
; check all filemtime() of original js files to check if minify's cache should be generated again.
; Should be set to "off" on production servers (i.e. manual empty cache needed when a file is changed) :
minifyCheckCacheFiletime = on # [on|off] : check whether at least one of the source files are newer than the cached files.
; list of filenames (no path) which shouldn't be minified - coma separated :
minifyExcludeCSS = "file1.css,file2.css"
minifyExcludeJS = ""
; add a unique ID to CSS and/or JS files URLs ( this gives for exemple /file.js?1267704635 ).
; This ID is actually the filemtime of each served file :
jsUniqueUrlId = off # [on|off] : add a tag as an url argument to JS files (whatever minifyJS)
cssUniqueUrlId = off # [on|off] : add a tag as an url argument to CSS files (whatever minifyCSS)

comment:9 Changed 5 years ago by bricet

  • review set to review?

comment:10 Changed 5 years ago by bibo

Well done bricet,

I should note that bricet is a colleague and i have already reviewed his code. I just have a couple of nits to add.

I suugest slightly different names s/outputCSSLinkTags/outputCSSLinks s/outputJSScriptTags/outputJSScripts to differentiate from the helper functions outputCSSLinkTag and outputJSScriptTag which really output one markup element.

Add a comment above "foreach ( minify ...)" lines saying why it can return more than one result. Add a comment above last block of outputCSSLinkTags and outputJSScriptTags to tell what it does ie. compress and minify pending files.

With this patch, two/three web rules for perf should be made easier for jelix developers :

  • minimize request number (css and js) and still being able to explode code logically between more files
  • compress response
  • easy way to add far future expires http header on files while still being able to develop through regular iterations.

Changed 5 years ago by bricet

Yet another bug about jsUniqueUrlId and cssUniqueUrlId. Really, really sorry ...

Changed 5 years ago by bricet

New patch applying bibo's comments

Changed 5 years ago by bricet

Yet yet another bug solved (small fix)

comment:11 Changed 5 years ago by laurentj

  • Status changed from new to closed
  • review changed from review? to review+
  • Resolution set to fixed
  • Milestone set to Jelix 1.2 beta

comment:12 Changed 5 years ago by laurentj

  • Documentation needed set
Note: See TracTickets for help on using tickets.