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

Made the java binary location customizable #47

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gfreeau
Copy link

@gfreeau gfreeau commented Aug 19, 2014

Closure compiler docs (https://code.google.com/p/closure-compiler/wiki/FAQ) recommend the use of 32bit JVM for performance improvements.

I was able to improve my project compile time from 70 seconds to 15 seconds by using their recommendations. The problem is the java binary is hard coded to use your system default.

This is not ideal because most people would be running a 64bit JVM and the 32bit JVM would only be used for a specific purpose, so trying to run a 32 bit JVM alongside a 64bit one would take a lot of tinkering.

With this patch you can specify this in the grunt task config:

{
    javaBin: '/opt/jdk1.8.0_20/bin/java -client -d32',
    closurePath: ...,
}

You can download any JDK and place it somewhere on the system and without setting any environment variables or editing anything etc, you can use it just for doing the compiling to gain the speed increase.

Also, is there any reason this.options() is not used in the task? Seems it would be more efficient for this.

@gmarty
Copy link
Owner

gmarty commented Aug 20, 2014

Looks great! Thanks a lot. Would you mind to add a quick documentation in the README.md file, just under the paragraph about closurePath?

Then I'll merge it.

@gfreeau
Copy link
Author

gfreeau commented Aug 21, 2014

I made some docs for this setting.

Did you see my comment about this.options()? There are a few settings for this task that would benefit from being set globally:

i.e

grunt.initConfig({
  'closure-compiler': {
    options: {
      javaBin: '/opt/jdk1.8.0_20/bin/java -client -d32',
      closurePath: '/src/to/closure-compiler', 
    }
    frontend: {
      js: 'static/src/frontend.js',
      jsOutputFile: 'static/js/frontend.min.js',
      options: {
        compilation_level: 'ADVANCED',
      }
    }
  }
});

@jarabek
Copy link

jarabek commented Jan 8, 2015

Could someone tell me when this PR is going to be merged? As this would be a pretty helpful addition.

@y-a-v-a
Copy link

y-a-v-a commented Jan 22, 2015

+1

@tuckbick
Copy link

tuckbick commented Jun 1, 2016

Wow, this was so close to being merged...

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

Successfully merging this pull request may close these issues.

5 participants