-
Notifications
You must be signed in to change notification settings - Fork 706
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
Adjust JIT memory usage based on cgroup limits #1371
Comments
Didn't we discuss a similar API point in relation to the GC? I vaguely recall something about having a single API and passing in an enum to indicate if it should be the cgroup limit or the system limit. |
@DanHeidinga I remember that discussion. It happened when we wanted only GC, and no other component, to use cgroup limits if a specific option is specified. GC would have to decide which limit, cgroup or system, to use and pass an enum to the portlibrary API. |
@DanHeidinga your thoughts? |
I'm hoping to give this more this week. I haven't had a chance to sit down and really think about the implications of this yet. |
If there is only going to be one place where we need the results from both, I suggest that location should make both calls. An API should only be added when there is a reasonable expectation that multiple callers will want the need the info. |
@DanHeidinga I believe you are referring to |
I agree with Ashu's comments above:
|
So we'd have to add a new api either way? That changes my answer. One of my concerns with the approach is that OMR already has 3 memory-related APIs that appear to have a high degree of overlap:
Adding a 4th won't help this situation. What about extending the In the non-cgroup case, these new fields would be assigned the same value as the original fields. They would only potentially differ if the cgroup limits are enabled. |
Your proposal about extending the J9MemoryInfo sounds good to me. |
@DanHeidinga @mpirvu I have added only those fields to |
Why the limitation on Linux?
Correct, if I have the values for the entire machine I can implement the required logic in a JIT function. We can export that later if there are other consumers. |
Because the cgroup is a Linux thing and that's where we would need to store host memory stats separately. On other systems, these new fields are not really required. |
Dan has also asked it to be platform neutral. I will update the code. |
@mpirvu fyi - PR eclipse/omr#2430 is merged now. |
Thanks, I am working on it. |
We have been using Is there any workaround while this issue continues to be debated? Thanks. |
This issue was fixed by PR #2546 Apart from the Java heap and the memory used by the JIT compiler, the JVM uses memory for other components as well: classes and other VM data structures, GC data structures, JIT persistent/runtime data structures. Any of these could have pushed the JVM over the container limits. |
Thanks @mpirvu - do you know if this fix is within the container I have?
It is hard to track back... |
Yes sr5fp25 has this fix. |
Thanks for that @mpirvu. So, given that we're using the fix that you referenced, here's a report from the Linux OOM reaper when things die for us:
It looks as though the problem occurs during a JIT on startup. Here are the options we're using on startup:
|
Rather than continue describing my issue here, I've now opened up a new one so that it can be discussed in a stand-alone manner: #4050 |
Portlibrary API for getting memory limit imposed by cgroup are now available in OMR and OpenJ9 has an option to enable cgroups which will cause GC to size its heap properly when -Xmx is not specified.
In addition to GC, @mpirvu suggested JIT should also be using cgroup memory limits to size its usage.
Comment from Marius:
Its interesting that
omrsysinfo_get_memory_info()
also determines total memory asomrsysinfo_get_physical_memory()
does but they use different mechanism.Given that we want to update portlibrary APIs to use cgroup limits transparently, I think
omrsysinfo_get_memory_info()
should be updated on same lines asomrsysinfo_get_physical_memory()
has been done, and should return values based on cgroup limits, if available, transparently.If we do that, then all the memory stats that
omrsysinfo_get_memory_info()
calculates would then be in context of the cgroup and not of the whole system when JVM is in a cgroup. After this is done, JIT can continue usingomrsysinfo_get_memory_info()
in the same way as it is being used.However, as Marius mentioned, even when we are running in a cgroup, we do need to take into account system wide value to get the correct value of available memory. This is because cgroup does not provide any mechanism to reserve memory, it just puts upper limit on the memory usage of the cgroup. This means other processes on the host (or other cgroups) can eat up into this cgroup's memory.
But if we update
omrsysinfo_get_memory_info()
to return cgroup stats as per current design, then there won't be any API that would return the system wide available memory.Given all these things, my proposal is:
omrsysinfo_get_memory_info()
to use memory stats exposed by memory controller of the cgroup.omrsysinfo_get_available_memory()
) to return available memory. This API would encapsulate the logic that Marius mentioned, that is return minimum of available memory in the system and available memory in the cgroup.omrsysinfo_get_memory_info()
.fyi - @DanHeidinga @charliegracie @pshipton
The text was updated successfully, but these errors were encountered: