Support for dynamically adjusting scratch memory space of compilations #2546

mpirvu · 2018-08-02T20:58:52Z

The JIT is allowed to use up to 256 MB of 'scratch' memory for a compilation
while JSR292 compilations increase this limit to 1.5 GB. These limits create
footprint spikes during an application run which can make resource
provisioning more cumbersome.

This commit adds the following changes:

The JIT will use portlib APIs to compute the amount of physical memory
available (taking into consideration any container limits). This value
is cached (to minimize API overhead) and recomputed with a desired
frequency (0.5 sec by default)
At the beginning of a compilation the JIT checks the amount of available
physical memory and sets a limit for the scratch space accordingly. If the
amount of physical memory is so low that a relatively cheap compilation cannot
be performed, the compilation request is aborted right there.
When a compilation thread needs to allocate a new scratch memory segment
(of size 16 MB) it checks again the available physical memory. If this is low,
the compilation is aborted and will be retried at a lower optimization level.
At the same time a flag will be set which will make one compilation thread
to suspend itself (however, the JIT always makes sure there is at least one
compilation thread active).

Signed-off-by: Marius Pirvu mpirvu@ca.ibm.com

fjeremic · 2018-08-02T21:01:19Z

@dsouzai FYI for review.

mpirvu · 2018-08-02T21:09:28Z

Note that at the moment the code is not ready to be merged (hence the WIP heading).
Also, the fix from eclipse-omr/omr#2785 is needed for this commit to work correctly.

mpirvu · 2018-08-02T21:11:00Z

@ashu-mehra FYI for review

dsouzai · 2018-08-03T20:47:29Z

runtime/compiler/control/CompilationThread.cpp

+// Compute free physical memory taking into account container limits
+// To limit overhead, this function caches the computed value and
+// only refreshes it if more than 0.5 sec have passed since the last update
+// or if "now" parameter is true


Where is the "now" parameter mentioned in this comment?

dsouzai · 2018-08-03T21:09:57Z

The changes lgtm.

runtime/compiler/control/CompilationThread.cpp

fjeremic · 2018-09-12T17:17:40Z

@mpirvu what is the state of this PR?

mpirvu · 2018-09-12T18:04:43Z

I just had a stab at it last night but in my local experiments I found the j9sysinfo_get_memory_info API to give the same results for host and container memory (which is wrong). I had a discussion with Ashutosh and the API seems to be working fine for him. Now it's on me to determine why j9sysinfo_get_memory_info gives me bad information.

mpirvu · 2018-09-13T04:27:53Z

With some instrumentation I determined that
rc = readCgroupSubsystemFile(portLibrary, OMR_CGROUP_SUBSYSTEM_MEMORY, CGROUP_MEMORY_SWAP_LIMIT_IN_BYTES_FILE, numItemsToRead, "%lu", &cgroupMemInfo->memoryAndSwapLimit);
returns an error code and therefore retrieveLinuxCgroupMemoryStats returns error even though the memoryLimit and memoryUsage were correctly read.

@ashu-mehra
In my opinion retrieveLinuxCgroupMemoryStats should always return 'success' and the validity of various memory statistics can be determined by comparison with OMRPORT_MEMINFO_NOT_AVAILABLE

mpirvu · 2018-09-13T04:30:30Z

I should also mention that on two Ubuntu 16.04 systems where I tried, when launching a container with a memory limit I get this warning:
WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.

mpirvu · 2018-09-14T21:45:59Z

Jenkins test all all jdk8

mpirvu · 2018-09-14T21:47:55Z

Could I get another review please: @ashu-mehra @dsouzai

dsouzai · 2018-09-17T18:29:26Z

runtime/compiler/control/CompilationThread.cpp

+      if (memInfo.buffered != OMRPORT_MEMINFO_NOT_AVAILABLE)
+         freePhysicalMemory += memInfo.buffered;
+      else
+         incomplete = !_cgroupMemorySubsystemEnabled;


Is it possible for incomplete to be true from the line 10510, but then set to false in this line? If so, Shouldn't there be a check to not set it to false?

You are right. I need to correct the behavior to never change it to 'false' once we set it to 'true'

dsouzai

Overall LGTM

ashu-mehra · 2018-09-18T11:02:00Z

runtime/compiler/env/J9SegmentAllocator.cpp

+            {
+            // Set a flag that will determine one compilation thread to suspend itself
+            // Even if multiple threads enter his path we still want to suspend only
+            // on thread, not several. This is why we use a flag and not a counter.


typo: on -> one

ashu-mehra · 2018-09-18T11:04:03Z

Looks fine to me.

ashu-mehra · 2018-09-18T11:50:03Z

In my opinion retrieveLinuxCgroupMemoryStats should always return 'success' and the validity of various memory statistics can be determined by comparison with OMRPORT_MEMINFO_NOT_AVAILABLE

I think we also need to set memoryAndSwapLimit to 0 if the file does not exist. And if we do that then that change alone will fix the issue.

The JIT is allowed to use up to 256 MB of 'scratch' memory for a compilation while JSR292 compilations increase this limit to 1.5 GB. These limits create footprint spikes during an application run which can make resource provisioning more cumbersome. This commit adds the following changes: 1) The JIT will use portlib APIs to compute the amount of physical memory available (taking into consideration any container limits). This value is cached (to minimize API overhead) and recomputed with a desired frequency (0.5 sec by default) 2) At the beginning of a compilation the JIT checks the amount of available physical memory and sets a limit for the scratch space accordingly. If the amount of physical memory is so low that a relatively cheap compilation cannot be performed, the compilation request is aborted right there. 3) When a compilation thread neeeds to allocate a new scratch memory segment (of size 16 MB) it checks again the available physical memory. If this is low, the compilation is aborted and will be retried at a lower optimization level. At the same time a flag will be set which will make one compilation thread to suspend itself (however, the JIT always makes sure there is at least one compilation thread active). 4) If the amount of free physical memory is low, the JIT will not activate additional compilation threads which can make the memory shortage even worse. 'low' phyical memory is determined by TR::Options::getSafeReservePhysicalMemoryValue() which currently defaults to 50MB and can be changed with -Xjit:safeReservePhysicalMemoryValue=<nnn> value in KB Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>

fjeremic · 2018-09-18T14:11:58Z

Jenkins test sanity all JDK8

fjeremic added the comp:jit label Aug 2, 2018

mpirvu force-pushed the autoscratchlimit branch from f08383d to 49d1af5 Compare August 2, 2018 21:06

dsouzai reviewed Aug 3, 2018

View reviewed changes

ashu-mehra reviewed Aug 20, 2018

View reviewed changes

runtime/compiler/control/CompilationThread.cpp Show resolved Hide resolved

fjeremic self-assigned this Aug 20, 2018

mpirvu force-pushed the autoscratchlimit branch from 49d1af5 to 9e4a340 Compare September 14, 2018 16:20

mpirvu changed the title ~~WIP: Support for dynamically adjusting scratch memory space of compilations~~ Support for dynamically adjusting scratch memory space of compilations Sep 14, 2018

fjeremic mentioned this pull request Sep 17, 2018

Test-Extended-JDK8-aix_ppc-64_cmprssptrs startupShutdownRobustness IOException: Error deleting #2141

Closed

dsouzai reviewed Sep 17, 2018

View reviewed changes

dsouzai approved these changes Sep 17, 2018

View reviewed changes

ashu-mehra reviewed Sep 18, 2018

View reviewed changes

mpirvu force-pushed the autoscratchlimit branch from 9e4a340 to e779d12 Compare September 18, 2018 13:16

fjeremic merged commit 5310edf into eclipse-openj9:master Sep 19, 2018

mpirvu mentioned this pull request Dec 9, 2018

Adjust JIT memory usage based on cgroup limits #1371

Closed

mpirvu deleted the autoscratchlimit branch December 13, 2018 15:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for dynamically adjusting scratch memory space of compilations #2546

Support for dynamically adjusting scratch memory space of compilations #2546

mpirvu commented Aug 2, 2018

fjeremic commented Aug 2, 2018

mpirvu commented Aug 2, 2018

mpirvu commented Aug 2, 2018

dsouzai Aug 3, 2018

dsouzai commented Aug 3, 2018

fjeremic commented Sep 12, 2018

mpirvu commented Sep 12, 2018

mpirvu commented Sep 13, 2018 •

edited

Loading

mpirvu commented Sep 13, 2018

mpirvu commented Sep 14, 2018

mpirvu commented Sep 14, 2018

dsouzai Sep 17, 2018

mpirvu Sep 17, 2018

dsouzai left a comment

ashu-mehra Sep 18, 2018

mpirvu Sep 18, 2018

ashu-mehra commented Sep 18, 2018

ashu-mehra commented Sep 18, 2018

fjeremic commented Sep 18, 2018

Support for dynamically adjusting scratch memory space of compilations #2546

Support for dynamically adjusting scratch memory space of compilations #2546

Conversation

mpirvu commented Aug 2, 2018

fjeremic commented Aug 2, 2018

mpirvu commented Aug 2, 2018

mpirvu commented Aug 2, 2018

dsouzai Aug 3, 2018

Choose a reason for hiding this comment

dsouzai commented Aug 3, 2018

fjeremic commented Sep 12, 2018

mpirvu commented Sep 12, 2018

mpirvu commented Sep 13, 2018 • edited Loading

mpirvu commented Sep 13, 2018

mpirvu commented Sep 14, 2018

mpirvu commented Sep 14, 2018

dsouzai Sep 17, 2018

Choose a reason for hiding this comment

mpirvu Sep 17, 2018

Choose a reason for hiding this comment

dsouzai left a comment

Choose a reason for hiding this comment

ashu-mehra Sep 18, 2018

Choose a reason for hiding this comment

mpirvu Sep 18, 2018

Choose a reason for hiding this comment

ashu-mehra commented Sep 18, 2018

ashu-mehra commented Sep 18, 2018

fjeremic commented Sep 18, 2018

mpirvu commented Sep 13, 2018 •

edited

Loading