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

Setting vmem is mostly useless, if java heap memory is your problem due to java settings #66

Open
tarrox opened this issue Apr 17, 2018 · 3 comments

Comments

@tarrox
Copy link
Collaborator

tarrox commented Apr 17, 2018

Currently erna process scripts allow to set the vmem of the qsub command if desired.
Problem is that setting this is useless for java due to the fact that the java command is set with -XX:MaxHeapSize=1024m ( in function assemble_facttools_call in file erna/utils.py line 68).
This means if you need more memory for java, setting vmem higher means nothing for the java process.

It is only usefull to the encapsulating python process, which is still useful, but gives the overall wrong impression of how to increase the memory if it is not enough for java. Which is a problem if you need more heap memory for your java problem, like me.
Also it seems that if the heap memory is not enough, it often leads to a heavy slowdown instead of a crash. Which is bad in itself, as it makes error searching a pain as it hides the real problem and makes you look elsewhere.

This needs some kind of fix.
We should either increase the maximum for the heap or there should be a way to set the max java heap memory somehow (Either as a percentage of vmem or directly trough a click option).

@maxnoe
Copy link
Member

maxnoe commented Apr 17, 2018

Would something like 0.9 * vmem work nicely?

@tarrox
Copy link
Collaborator Author

tarrox commented May 18, 2018

It would work but from my understanding the java (without the heap) and python process have a rather static memory footprint so setting it to vmem - staticFootPrint would be better. Only issue is to figure out a good value for staticFootPrint. 1 GB should be enough, so setting statitcFootPrint = 1GB should work as a quick and dirty solution. Also means that the minimum for vmem should be around 2 GB.

@tarrox
Copy link
Collaborator Author

tarrox commented May 18, 2018

Or as a real quick and dirty solution we remove the option for now as the default maxheapsize sould be higher on a 64bit machine than our current maximal need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants