#1143 ✓ checked-in
David Humphrey

Pick one minifier

Reported by David Humphrey | March 11th, 2011 @ 12:21 PM | in 1.2

Currently we are minifying our code twice, with YUI Compressor and also Google's Closure Compiler. Closure makes smaller code, and I'd like to see us move that direction if at all possible. We've had some issues in the past with subtle bugs in the code they generate. I'm also somewhat hesitant to let them rewrite our code and potentially introduce perf issues. This needs some thought.

Comments and changes to this ticket

  • Lonnen

    Lonnen March 11th, 2011 @ 02:44 PM

    I am concerned about the 'subtle bugs'. Are these things that would be caught if we ran the compressed version through the automated tests?

  • David Humphrey

    David Humphrey March 11th, 2011 @ 02:56 PM

    Yes, we'll catch that kind of bug. But we have our code tuned to run fast already, and if they rewrite our code, we could (not will) lose some of that. Note that we don't have to use the Advanced Rewriting Rules.

  • Lonnen

    Lonnen March 11th, 2011 @ 03:08 PM

    I'm wondering what kind of performance loss we're looking at. It seems like this could benefit from a resolution of issue #663.

  • David Humphrey

    David Humphrey March 11th, 2011 @ 03:10 PM

    Yeah, fixing #663 is really what we need to do here. There's no other easy way to be sure, except to have some data.

  • annasob

    annasob April 15th, 2011 @ 12:58 PM

    • State changed from “bugs” to “assigned”
    • Assigned user set to “Lonnen”
    • Milestone order changed from “18” to “0”

    I am hoping this goes in hand with #494 :)

  • Lonnen

    Lonnen May 10th, 2011 @ 03:02 PM

    • Assigned user changed from “Lonnen” to “Jon Buckley”
    • Milestone order changed from “26” to “0”
  • Lonnen

    Lonnen May 10th, 2011 @ 03:05 PM

    • Assigned user changed from “Jon Buckley” to “Lonnen”
  • Lonnen

    Lonnen May 10th, 2011 @ 04:29 PM

    Just ran perf tests against uncompressed, yui, and closure compressed source. I used Dhodgin's review-663 branch for the perf tests on an I7 MBP. All tests were run with 100 iterations, and completed 47/48 in the appropriate time in similar runtimes. The biggest difference was that closure compressed code was about 2/3 the size of the YUI compressed code.

    FF4.0.1

    Compression    Size          Runtime
    ----------------------------------------------
    None           721 KB        21325 ms
    YUI            299 KB        21065 ms
    Closure        205 KB        21167 ms
    

    Based on this I can recommend Closure, though its a good idea to run our tests against the compressed version to make sure that the name munging doesn't introduce a new bug.

  • Jon Buckley

    Jon Buckley May 11th, 2011 @ 11:40 AM

    Right now the Closure compiler throws many warnings about bad JSDoc @param's. It's possible to add command line switches to ignore these errors, but I think it'd be better to fix the JSDoc so that it parses correctly. Try running "make package-sketch SKETCHINPUT=example.pjs" and you'll get some of the following errors:

    example.pjs.js.src:622: WARNING - Parse error. expected closing }
           * @returns {Object[]} Returns an array containing all of the elements in this list in the correct order
                             ^
    
    example.pjs.js.src:2855: WARNING - Parse error. duplicate variable name "parent"
         * @param {PShapeSVG} parent   the parent PShapeSVG
                              ^
    
    example.pjs.js.src:3111: WARNING - Suspicious code. This code lacks side-effects. Is there a bug?
          } else if (name === "radialGradient") {
                 ^
    
    example.pjs.js.src:3113: WARNING - Suspicious code. This code lacks side-effects. Is there a bug?
          } else if (name === "linearGradient") {
                 ^
    
    example.pjs.js.src:3115: WARNING - Suspicious code. This code lacks side-effects. Is there a bug?
          } else if (name === "text") {
                 ^
    
    example.pjs.js.src:3118: WARNING - Suspicious code. This code lacks side-effects. Is there a bug?
          } else if (name === "filter") {
                 ^
    
    example.pjs.js.src:3120: WARNING - Suspicious code. This code lacks side-effects. Is there a bug?
          } else if (name === "mask") {
                 ^
    
    example.pjs.js.src:4509: WARNING - Parse error. expected closing }
           * @param {String[]} items   result of splitting the query on slashes
                           ^
    
    example.pjs.js.src:4509: WARNING - Parse error. expecting a variable name in a @param tag
           * @param {String[]} items   result of splitting the query on slashes
                           ^
    
    example.pjs.js.src:4534: WARNING - Parse error. expected closing }
           * @param {String[]} items   result of splitting the query on slashes
                           ^
    
    example.pjs.js.src:4534: WARNING - Parse error. expecting a variable name in a @param tag
           * @param {String[]} items   result of splitting the query on slashes
                           ^
    
    example.pjs.js.src:4537: WARNING - Parse error. expected closing }
           * @return {XMLElement[]} matching elements or empty array if no match
                                ^
    
    example.pjs.js.src:4557: WARNING - Parse error. expected closing }
           * @param {XML document childNodes} elementpath    the remaining nodes that need parsing
                         ^
    
    example.pjs.js.src:4609: WARNING - Parse error. expected closing }
           * @return {String[]} a list of element names.
                            ^
    
    example.pjs.js.src:4818: WARNING - Parse error. expected closing }
           * @param {float[]} elements    an array of elements to set this matrix to
                          ^
    
    example.pjs.js.src:4818: WARNING - Parse error. expecting a variable name in a @param tag
           * @param {float[]} elements    an array of elements to set this matrix to
                          ^
    ...
    0 error(s), 92 warning(s)
    
  • David Humphrey

    David Humphrey May 11th, 2011 @ 04:04 PM

    • State changed from “assigned” to “peer-review-requested”

    I agree with Lonnen, let's go with Closure.

    https://github.com/humphd/processing-js/tree/t1143
    https://github.com/humphd/processing-js/commit/f713255175e28a7b73f6...

    I've left YUI in, but turned it off (e.g., release no longer uses it).

  • annasob

    annasob May 14th, 2011 @ 01:45 AM

    • State changed from “peer-review-requested” to “super-review-requested”
    • Assigned user changed from “Lonnen” to “Jon Buckley”

    This looks good!

    • ran a couple of make commands and verified that the minified created works with sketches
    • What do we do about the errors Im up form making a new ticket

    PR+

  • Jon Buckley

    Jon Buckley May 14th, 2011 @ 10:27 AM

    Yeah, lets go with a separate ticket for the errors

  • annasob

    annasob May 16th, 2011 @ 10:18 AM

    • State changed from “super-review-requested” to “review-looks-good”

    I created a new ticket #1243 for the errors. Im also assuming Jon reviewed this so setting to R+

  • Jon Buckley

    Jon Buckley May 16th, 2011 @ 11:12 AM

    • Assigned user changed from “Jon Buckley” to “annasob”

    Yes, sorry, SR+.

  • annasob

    annasob May 16th, 2011 @ 11:24 AM

    • State changed from “review-looks-good” to “staged”
    • Assigned user changed from “annasob” to “Jon Buckley”

    Staged in annasob/processing-js commit

  • Jon Buckley

    Jon Buckley June 4th, 2011 @ 11:50 PM

    • State changed from “staged” to “checked-in”
    • Milestone order changed from “27” to “0”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Processing.js is an open programming language for people who want to program images, animation, and interactions for the web without using Flash or Java applets. Processing.js uses Javascript to draw shapes and manipulate images on the HTML5 Canvas element. The code is light-weight, simple to learn and makes an ideal tool for visualizing data, creating user-interfaces and developing web-based games.

Shared Ticket Bins

Referenced by

Pages