Adjust the max heap memory to the max ram installed #1713
Conversation
| break; | ||
| } | ||
| long memSize = Math.min(parmSize,((com.sun.management.OperatingSystemMXBean) ManagementFactory.getOperatingSystemMXBean()).getTotalPhysicalMemorySize()); | ||
| heapArgs.add("-Xmx"+memSize/1024+"K"); |
There was a problem hiding this comment.
I think -Xms need to be handled here too, right?
There was a problem hiding this comment.
I don't know if we should treat -Xms in the same way. If some user pass this parameter intentionally, he wants to make some performance tunning, and for that he should know the installed memory and tune this parameter accordingly, and if wrongly tuned in a way that leads to OOM or IPED halting, he should "feel" the effect and correct it by himself.
BTW, I will include some code to inform that the change in -Xmx has occured in App LOG.
There was a problem hiding this comment.
I think that we could change the -Xmx to half the installed memory only if it is greater than the total memory, so the user have a little "space" to increase by himself this value. What do you think?
There was a problem hiding this comment.
I think that we could change the -Xmx to half the installed memory only if it is greater than the total memory, so the user have a little "space" to increase by himself this value. What do you think?
It seems a good option.
There was a problem hiding this comment.
I don't know if we should treat -Xms in the same way. If some user pass this parameter intentionally, he wants to make some performance tunning, and for that he should know the installed memory and tune this parameter accordingly, and if wrongly tuned in a way that leads to OOM or IPED halting, he should "feel" the effect and correct it by himself.
The problem is that in line 75 -Xms is already handled, so the current code would transform a -Xms## into a -Xmx##, which I guess is not the desired behavior, right?
About changing or not -Xms, the proposed idea is to change a -Xmx, even if it was passed by the user, if it exceeds the total physical memory, right? Wouldn't the same reasoning apply to -Xms?
There was a problem hiding this comment.
Oh yes. I didn't noted. I put it there for some testing reason. I will remove.
There was a problem hiding this comment.
About the same reasoning: The best option, I think, would be not to change the user chosen passed value via cmd line and let him evaluate the effects.
But we need to change somewhere the default max heap memory. The "exe" loads with "-Xmx30G", and this value should change. Maybe we could change the code that declares this "-Xmx30G". But I didn't find where is it. @lfcnassif , could we point out where this value of 30G is specified?
There was a problem hiding this comment.
The "exe" loads with "-Xmx30G", and this value should change. Maybe we could change the code that declares this "-Xmx30G". But I didn't find where is it. @lfcnassif , could we point out where this value of 30G is specified?
It is hard coded into the EXE, I'll remove it and leave it blank (no heap option).
About current changes, my proposal is to use half the physical memory as Xmx if it is not specified by user. If it is specified and if it is greater than physical memory, cut it to be equal to physical memory. Seems reasonable?
There was a problem hiding this comment.
I put it there for some testing reason. I will remove.
Xms was already there before, it should be passed to the second process if specified by user. And I agree to @tc-wleite, if specified and greater than physical memory, limiting it to physical memory seems reasonable to me.
There was a problem hiding this comment.
And even if Xms/Xmx are specified by user, I think we can limit them to physical memory, allocating more makes no sense to me. That's why I tagged the issue as bug. Like current EXE, users may create and distribute scripts to open multicases and specify fixed heap values which may be bad depending on the runtime machine, so I think limiting to physical memory is a good approach to workaround user mistakes.
value is greater than the installed RAM. Removes wrong -Xms parameter processing.
|
What about using 1/4 the RAM when processing if -Xmx is not specified (current behavior) and limiting to 32GB? We have image reading processes and could have parsing processes running concurrently. 1/2 RAM would be used just at analysis time. |
There was a problem hiding this comment.
Thanks @patrickdalla for this, and thanks @tc-wleite for reviewing!
Adjust the max heap memory to the max ram installed if the -Xmx parameter is larger than this. Closes #1712