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

Update jar_installer.rb #7

Merged
merged 1 commit into from Sep 18, 2014

Conversation

Projects
None yet
2 participants
@ericachelis
Copy link
Contributor

commented Sep 16, 2014

The IO.read method reads the entire JAR file into RAM. In our environment the jar file is roughly 900MB. This was causing Out of Memory exceptions on our machines.

This proposed change will leverage the remote_file resource and the source attribute. The file is still read from the local cache directory. The file is buffered much more effectively when taking this approach.

I also propose updating the action to only create the file if it is missing.

This change reduced the memory consumption of this resource by more than an order of magnitude. This change also reduced the execution duration for this resource by roughly 10 seconds in our environment.

Please let me know if you have any questions.

Update jar_installer.rb
The IO.read method reads the entire JAR file into RAM.  In our environment the jar file is roughly 900MB.  This was causing Out of Memory exceptions on our machines.

This proposed change will leverage the remote_file resource and the source attribute. The file is still read from the local cache directory. The file is buffered much more effectively when taking this approach.

I also propose updating the action to only create the file if it is missing.

This change reduced the memory consumption of this resource by more than an order of magnitude. This change also reduced the execution duration for this resource by roughly 10 seconds in our environment.

Please let me know if you have any questions.
@pdunnavant

This comment has been minimized.

Copy link
Collaborator

commented Sep 16, 2014

Awesome! Thanks! I'll take a look at this a little later today.

@pdunnavant

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2014

👍 I ran all the tests, everything looks great. Thanks again for your contribution!

pdunnavant added a commit that referenced this pull request Sep 18, 2014

@pdunnavant pdunnavant merged commit c2f7d2b into tacitknowledge:master Sep 18, 2014

@ericachelis ericachelis deleted the ericachelis:patch-1 branch Sep 19, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.