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

use J as a binary instead of loading the module ( 120 + MBs ) #22

Merged
merged 1 commit into from
Jul 29, 2014

Conversation

davidworkman9
Copy link
Contributor

Addresses issue #21. Slower than before (1), but avoids loading over 120 MB into memory.

I looked for a way to "unload" a module, so we could include it only at "extract" time then unload it when finished with it but everything that I was able to find didn't free up the memory.

  1. I benchmarked around 120 ms before, 775 ms now on an i5-3570K 3.4 GHz, 3.8 turbo.

dbashford added a commit that referenced this pull request Jul 29, 2014
use J as a binary instead of loading the module ( 120 + MBs )
@dbashford dbashford merged commit 460af67 into dbashford:master Jul 29, 2014
@dbashford
Copy link
Owner

I'm cool with this. Will wait and see if anyone complains about response time.

@SheetJSDev
Copy link

@dbashford You should also update the j dependency as the new module is significantly smaller in size and memory footprint.

@dbashford
Copy link
Owner

@davidworkman9 if you get a second, could you give that a shot? I could, but it would be apples v apples given your memory issues. I'd be curious what the improvement would be vs the 10 -> 135 you mentioned in #21.

@davidworkman9
Copy link
Contributor Author

Sorry I took so long, I've been without power at my office for the past 2 days. I tried the latest verison of J and memory usage dropped down to around 70 MB. So about half but still pretty high IMO.

@dbashford
Copy link
Owner

👍

May go back in some day and allow toggling between two options, one for the speed conscious and another for the memory conscious.

Thanks for getting back!

@tommygnr
Copy link
Contributor

tommygnr commented May 23, 2016

@dbashford this change doesn't play nicely with npm v3 flattening dependencies loaded as a dependency in another project

@dbashford
Copy link
Owner

Yeah, so instead of J being at

projectRoot/node_modules/textract/node_modules/J

It is at

projectRoot/node_modules/J/?

@dbashford
Copy link
Owner

dbashford commented May 23, 2016

Curious, @tommygnr, in that situation, is there a j.njs inside node_modules/.bin?

@tommygnr
Copy link
Contributor

@dbashford to your first question: Yes j is at projectRoot/node_modules/J/
Your second question is a little ambiguous. There is no node_modules folder inside textract but there is a projectRoot/node_modules/.bin/j which is symlinked to the j.njs file

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.

4 participants