Implement initial RAM class persistence feature #20387

ThanHenderson · 2024-10-18T21:17:03Z

This PR revives RAM class persistence work. It currently rebases/implements: https://github.ibm.com/runtimes/openj9-stratum/pull/7 and https://github.ibm.com/runtimes/openj9-stratum/pull/25.

Current considerations/limitations:

No JIT support (i.e. -Xint required)
Incompatible with SCC (i.e. -Xshareclasses:none required)
ASLR must be turned off (I've been using echo 0 | sudo tee /proc/sys/kernel/randomize_va_space to turn this off)
Build must be configured with --enable-snapshots (requires extension repo patches)
No functionality to auto-detect whether a specified cache is compatible with the current invocation. (i.e. if you specify ramCache as the cache name for one program, then again for a different program, it will try to reuse the created cache rather than recognizing that it is incompatible and emitting a warning or deleting it and creating a new one).
Development only for JDK8, need to reassess to see what is required for later versions.

Example:

# Ensure ASLR is disabled (echo 2 to reset to default)
echo 0 | sudo tee /proc/sys/kernel/randomize_va_space

# Run the java program, creating a cache called "ramCache" that persists RAM class data
java -Xint -Xshareclasses:none -Xsnapshot=ramCache <java-program>

# Run the same java program, loading the persisted RAM class data from an image called "ramCache"
java -Xint -Xshareclasses:none -Xsnapshot=ramCache <java-program>

Signed-off-by: Nathan Henderson [email protected]

ThanHenderson · 2024-10-18T21:17:33Z

ping @babsingh

ThanHenderson · 2024-10-23T20:51:32Z

fyi @tajila @gacholio @hangshao0

runtime/vm/j9vm.tdf

keithc-ca · 2024-10-29T13:53:02Z

runtime/cmake/options.cmake

@@ -147,6 +147,8 @@ option(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES "Enables support for Project Va
 option(J9VM_OPT_ROM_IMAGE_SUPPORT "Controls if the VM includes basic support for linked rom images")
 option(J9VM_OPT_SHARED_CLASSES "Support for class sharing")

+option(J9VM_OPT_SNAPSHOTS "Support for RAM class snapshot images")


The equivalent flag is missing for UMA builds.

UMA flag is added in j9.flags. Here is an example:

openj9/buildspecs/j9.flags

Lines 1727 to 1730 in 17ee5e7

<flag id="opt_fips">

<description>Add supports for FIPs</description>

<ifRemoved></ifRemoved>

</flag>

<flag id="opt_snapshots"> <description>Enable support for RAM class snapshot images</description> <ifRemoved>Disable support for RAM class snapshot images</ifRemoved> </flag>

I think the name is a little too generic: "snapshots" also apply to CRaC and CRIU.
How about "opt_ram_class_snapshots"? (The cmake name would change accordingly.)

I'm fine with changing this to something else. But the generic-ness of snapshots is more amenable to expanding what data is held in a "snapshot" in the future.

keithc-ca · 2024-10-30T13:26:09Z

buildenv/jenkins/variables/defaults.yml

+#========================================#
+# RAM Persistence
+#========================================#
+snapshots:
+  extra_configure_options: '--enable-snapshots'


Without uses, this has no value, but it should be near other features.

keithc-ca · 2024-10-30T13:26:37Z

runtime/cmake/caches/common.cmake

@@ -196,6 +196,7 @@ set(J9VM_OPT_OPENJDK_METHODHANDLE OFF CACHE BOOL "")
 set(J9VM_OPT_REFLECT ON CACHE BOOL "")
 set(J9VM_OPT_ROM_IMAGE_SUPPORT ON CACHE BOOL "")
 set(J9VM_OPT_SHARED_CLASSES ON CACHE BOOL "")
+set(J9VM_OPT_SNAPSHOTS ON CACHE BOOL "")


I think the default should be "OFF" for now.

keithc-ca · 2024-10-30T13:27:52Z

runtime/gc_modron_startup/mminit.cpp

 	}

+


One blank line is sufficient.

keithc-ca · 2024-10-30T13:33:09Z

runtime/j9vm/jvm.c

 		}
+#if defined(J9VM_OPT_SNAPSHOTS)
+		else if (0 == strncmp(args->options[argCursor].optionString, VMOPT_XSNAPSHOT, strlen(VMOPT_XSNAPSHOT))) {
+			specialArgs->vmSnapshotFilePath = args->options[argCursor].optionString + strlen(VMOPT_XSNAPSHOT);
+		}
+#endif /* defined(J9VM_OPT_SNAPSHOTS) */


Suggest moving the closing brace after #endif:

#if defined(J9VM_OPT_SNAPSHOTS) } else if (0 == strncmp(args->options[argCursor].optionString, VMOPT_XSNAPSHOT, strlen(VMOPT_XSNAPSHOT))) { specialArgs->vmSnapshotFilePath = args->options[argCursor].optionString + strlen(VMOPT_XSNAPSHOT); #endif /* defined(J9VM_OPT_SNAPSHOTS) */ }

keithc-ca · 2024-10-30T13:35:24Z

runtime/jcl/common/jcldefine.c

@@ -26,6 +26,7 @@
 #include "j9protos.h"
 #include "j9jclnls.h"
 #include "j9vmnls.h"
+#include "vm_api.h"


Why is this necessary?

keithc-ca · 2024-10-30T15:03:34Z

runtime/vm/jvmfree.c

+#if defined(J9VM_OPT_SNAPSHOTS)
+#include "j9port_generated.h"
+#endif /* defined(J9VM_OPT_SNAPSHOTS) */


Odd. I distinctly remember removing this in a previous revision. It must've slipped back in during a squash at some point. Thanks for catching that.

keithc-ca · 2024-10-30T15:09:14Z

runtime/vm/segment.c

-		} else {
+		}
+#if defined(J9VM_OPT_SNAPSHOTS)
+		else if (J9_ARE_ANY_BITS_SET(segment->type, MEMORY_TYPE_ROM_CLASS)
+			&& IS_SNAPSHOTTING_ENABLED(javaVM)
+		) {
+			vmsnapshot_free_memory(segment->baseAddress);
+		}
+#endif /* defined(J9VM_OPT_SNAPSHOTS) */
+		else {


Suggest relocating the braces:

#if defined(J9VM_OPT_SNAPSHOTS) } else if (J9_ARE_ANY_BITS_SET(segment->type, MEMORY_TYPE_ROM_CLASS) && IS_SNAPSHOTTING_ENABLED(javaVM) ) { vmsnapshot_free_memory(segment->baseAddress); #endif /* defined(J9VM_OPT_SNAPSHOTS) */ } else {

keithc-ca · 2024-10-30T15:10:59Z

runtime/vm/segment.c

+	/* TODO: In J9MemorySegmentList flags, add an option to signify allocation from the
+	 * VMSnapshotImpl to replace this check (See @ref omr:j9nongenerated.h).
+	 */
+	if (IS_SNAPSHOTTING_ENABLED(javaVM) && ((javaVM->classMemorySegments == segmentList) || (javaVM->memorySegments == segmentList))) {


Suggest folding this:

if (IS_SNAPSHOTTING_ENABLED(javaVM) && ((javaVM->classMemorySegments == segmentList) || (javaVM->memorySegments == segmentList)) ) {

keithc-ca · 2024-10-30T15:15:36Z

runtime/vm/j9vm.tdf

+TraceEntry=Trc_VM_snapshot_readSnapshotFromFile_Entry NoEnv Overhead=1 Level=1 Template="readSnapshotFromFile() Snapshot Heap = %p. File being read = %s"
+TraceExit=Trc_VM_snapshot_readSnapshotFromFile_Exit NoEnv Overhead=1 Level=1 Template="readSnapshotFromFile() Snapshot read successful"
+TraceEntry=Trc_VM_snapshot_writeSnapshotToFile_Entry NoEnv Overhead=1 Level=1 Template="writeSnapshotToFile() Snapshot Heap = %p. File being written to = %s"
+TraceExit=Trc_VM_snapshot_writeSnapshotToFile_Exit NoEnv Overhead=1 Level=1 Template="writeSnapshotTofile() Snapshot write successful"
+TraceEntry=Trc_VM_snapshot_subAllocateSnapshotMemory_Entry NoEnv Overhead=1 Level=1 Template="subAllocateMemory() Snapshot Header = %p. Byte amount = %d."
+TraceExit=Trc_VM_snapshot_subAllocateSnapshotMemory_Exit NoEnv Overhead=1 Level=1 Template="subAllocateMemory() Memory allocated = %p."
+TraceEvent=Trc_VM_snapshot_warmClassLoadFromSnapshot_ClassLoadHookFailed Overhead=1 Level=1 Template="loadWarmClassFromSnapshot() Warm class load hook failed class=%p, %s"
+TraceEvent=Trc_VM_snapshot_loadWarmClassFromSnapshot_ClassInfo Overhead=1 Level=1 Template="loadWarmClassFromSnapshot() LoadClass clazz=%p, %s"


Not all of these new tracepoints are actually used; please remove the unused ones.

keithc-ca · 2024-10-30T15:16:21Z

runtime/vm/j9vm.tdf

+TraceExit=Trc_VM_snapshot_readSnapshotFromFile_Exit NoEnv Overhead=1 Level=1 Template="readSnapshotFromFile() Snapshot read successful"
+TraceEntry=Trc_VM_snapshot_writeSnapshotToFile_Entry NoEnv Overhead=1 Level=1 Template="writeSnapshotToFile() Snapshot Heap = %p. File being written to = %s"
+TraceExit=Trc_VM_snapshot_writeSnapshotToFile_Exit NoEnv Overhead=1 Level=1 Template="writeSnapshotTofile() Snapshot write successful"
+TraceEntry=Trc_VM_snapshot_subAllocateSnapshotMemory_Entry NoEnv Overhead=1 Level=1 Template="subAllocateMemory() Snapshot Header = %p. Byte amount = %d."


%d should be %zu

dmitripivkine

Change in GC code looks ok. I hope for "restore" case buffers are cleared properly as well.

ThanHenderson · 2024-10-30T20:28:31Z

@keithc-ca I've addressed your comments. And rebased to resolve the merge conflicts.

Related: https://github.ibm.com/runtimes/openj9/issues/755 Related: eclipse-openj9/openj9#20387 Signed-off-by: Nathan Henderson <[email protected]>

keithc-ca · 2024-10-31T14:26:04Z

buildspecs/j9.flags

+	<flag id="opt_snapshots">
+		<description>Enable support for RAM class snapshot images</description>
+		<ifRemoved>Disable support for RAM class snapshot images</ifRemoved>
+	</flag>


Please keep flags sorted alphabetically (this should follow opt_sidecar).

keithc-ca · 2024-10-31T14:28:06Z

runtime/oti/j9nonbuilder.h

+#if defined(J9VM_OPT_SNAPSHOTS)
+/* Snapshot utility macros. */


Please swap these two lines (the comment relates to the #if ... #endif block.

/* Snapshot utility macros. */ #if defined(J9VM_OPT_SNAPSHOTS)

keithc-ca · 2024-10-31T14:47:58Z

runtime/vm/VMSnapshotImpl.hpp

+
+	bool setupRestoreRun();
+	bool setupSnapshotRun();
+	bool setupSnapshotRunPostInitialize(void);


Please remove void from all declarations of C++ methods with no parameters.

keithc-ca · 2024-10-31T16:22:43Z

runtime/gc_modron_startup/mminit.cpp

+#if defined(J9VM_OPT_SNAPSHOTS)
+	if (!IS_RESTORE_RUN(vm))
+#endif /* defined(J9VM_OPT_SNAPSHOTS) */
+	{


Now that macros like IS_RESTORE_RUN are always defined, code like this can be simplified:

if (!IS_RESTORE_RUN(vm)) {

See also near line 568, and similar uses in other files.

keithc-ca · 2024-10-31T16:26:27Z

runtime/oti/SnapshotFileFormat.h

+ * [1] https://www.gnu.org/software/classpath/license.html
+ * [2] http://openjdk.java.net/legal/assembly-exception.html
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception


This should be:

* [1] https://www.gnu.org/software/classpath/license.html * [2] https://openjdk.org/legal/assembly-exception.html * * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 OR GPL-2.0-only WITH OpenJDK-assembly-exception-1.0

keithc-ca · 2024-10-31T17:05:03Z

runtime/vm/VMSnapshotImpl.hpp

+ * [1] https://www.gnu.org/software/classpath/license.html
+ * [2] http://openjdk.java.net/legal/assembly-exception.html
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception


This should be

* [1] https://www.gnu.org/software/classpath/license.html * [2] https://openjdk.org/legal/assembly-exception.html * * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 OR GPL-2.0-only WITH OpenJDK-assembly-exception-1.0

keithc-ca · 2024-10-31T17:06:07Z

runtime/vm/VMSnapshotImpl.hpp

+/*
+* VMSnapshotImpl.hpp
+*/


Please fix the indentation here and add a blank line:

/* * VMSnapshotImpl.hpp */

keithc-ca · 2024-10-31T17:06:49Z

runtime/vm/VMSnapshotImpl.hpp

+#endif /* defined(J9VM_OPT_SNAPSHOTS) */
+#endif /* VMSNAPSHOTIMPLE_HPP_ */


Please add a blank line and fix the typo:

#endif /* defined(J9VM_OPT_SNAPSHOTS) */ #endif /* VMSNAPSHOTIMPL_HPP_ */

keithc-ca · 2024-10-31T17:08:50Z

runtime/oti/SnapshotFileFormat.h

+#include "j9cfg.h"
+
+#if defined(J9VM_OPT_SNAPSHOTS)
+


This uses a number of types that won't be declared (not even a forward declaration); some includes are needed, at least:

#include "j9nongenerated.h"

keithc-ca · 2024-10-31T17:09:16Z

runtime/oti/SnapshotFileFormat.h

+#endif /* defined(J9VM_OPT_SNAPSHOTS) */
+#endif /* snapshotfileformat_h */


Please add a blank line after line 135.

Related: https://github.ibm.com/runtimes/openj9/issues/755 Related: eclipse-openj9/openj9#20387 Signed-off-by: Nathan Henderson <[email protected]>

With the motivation of reducing startup time, this patch implements initial support for a mechanism to persist RAM classes to disk during a training run and load the persisted snapshot on subsequent identical production runs. Towards this goal, this patch: - Identifies an -Xsnapshot= command-line option that enables the snapshotting functionality and takes a string parameter that specifies a file path where the snapshot image is/will be. - Adds SnapshotFileFormat.h file that specifies the format of the snapshot image that is persisted to disk. It describes the J9JavaVM members that are supported, namely immortal J9ClassLoader objects and the J9Class objects that they load. - Adds the VMSnapshotImpl files that specify and implement the snapshotting API and include suballocators wherein the data to include within a snapshot is allocated. - Ensures correct allocation/initialization of the immortal J9ClassLoaders and memory segments snapshotting and restoring. - Enables taking a snapshot on VM shutdown, and restoring during startup. Co-authored-by: Tobi Ajila <[email protected]> Co-authored-by: Babneet Singh <[email protected]> Co-authored-by: Nathan Henderson <[email protected]> Signed-off-by: Nathan Henderson <[email protected]>

ThanHenderson force-pushed the monorcp branch 4 times, most recently from 34b63ce to 9a8932d Compare October 23, 2024 20:48

ThanHenderson marked this pull request as ready for review October 23, 2024 20:50

dmitripivkine added comp:vm comp:gc labels Oct 28, 2024

babsingh reviewed Oct 28, 2024

View reviewed changes

runtime/vm/j9vm.tdf Outdated Show resolved Hide resolved

This was referenced Oct 28, 2024

Add configure support to enable RAM class persistence ibmruntimes/openj9-openjdk-jdk8#770

Merged

Fix configure support for RAM class persistence ibmruntimes/openj9-openjdk-jdk8#773

Merged

keithc-ca reviewed Oct 29, 2024

View reviewed changes

ThanHenderson force-pushed the monorcp branch from 9a8932d to a80459c Compare October 30, 2024 14:14

keithc-ca requested changes Oct 30, 2024

View reviewed changes

dmitripivkine approved these changes Oct 30, 2024

View reviewed changes

ThanHenderson force-pushed the monorcp branch 2 times, most recently from 0c74fda to ebf6402 Compare October 30, 2024 20:27

ThanHenderson mentioned this pull request Oct 31, 2024

Add configure support to enable RAM class persistence ibmruntimes/openj9-openjdk-jdk#873

Merged

keithc-ca requested changes Oct 31, 2024

View reviewed changes

ThanHenderson force-pushed the monorcp branch from ebf6402 to 85a3135 Compare November 1, 2024 14:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement initial RAM class persistence feature #20387

Implement initial RAM class persistence feature #20387

ThanHenderson commented Oct 18, 2024 •

edited

Loading

ThanHenderson commented Oct 18, 2024

ThanHenderson commented Oct 23, 2024

keithc-ca Oct 29, 2024

babsingh Oct 29, 2024

keithc-ca Oct 29, 2024

ThanHenderson Oct 29, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

ThanHenderson Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

keithc-ca Oct 30, 2024

dmitripivkine left a comment

ThanHenderson commented Oct 30, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

keithc-ca Oct 31, 2024

	<flag id="opt_fips">
	<description>Add supports for FIPs</description>
	<ifRemoved></ifRemoved>
	</flag>

		#if defined(J9VM_OPT_SNAPSHOTS)
		/* Snapshot utility macros. */

		#endif /* defined(J9VM_OPT_SNAPSHOTS) */
		#endif /* VMSNAPSHOTIMPLE_HPP_ */

		#endif /* defined(J9VM_OPT_SNAPSHOTS) */
		#endif /* snapshotfileformat_h */

Implement initial RAM class persistence feature #20387

Are you sure you want to change the base?

Implement initial RAM class persistence feature #20387

Conversation

ThanHenderson commented Oct 18, 2024 • edited Loading

ThanHenderson commented Oct 18, 2024

ThanHenderson commented Oct 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitripivkine left a comment

Choose a reason for hiding this comment

ThanHenderson commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThanHenderson commented Oct 18, 2024 •

edited

Loading