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

8259070: Add jcmd option to dump CDS #2737

Closed
wants to merge 17 commits into from
Closed
Changes from 1 commit
Commits
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -47,7 +47,6 @@
#include "memory/archiveBuilder.hpp"
#include "memory/cppVtables.hpp"
#include "memory/dumpAllocStats.hpp"
#include "memory/dynamicArchive.hpp"
#include "memory/filemap.hpp"
#include "memory/heapShared.hpp"
#include "memory/metaspace.hpp"
@@ -4257,54 +4257,50 @@ bool InstanceKlass::is_shareable() const {

void InstanceKlass::log_to_classlist(const ClassFileStream* stream) const {
#if INCLUDE_CDS
ResourceMark rm;
if (ClassListWriter::is_enabled()) {
if (!ClassLoader::has_jrt_entry()) {
warning("DumpLoadedClassList and CDS are not supported in exploded build");
DumpLoadedClassList = NULL;
return;
}
ClassLoaderData* loader_data = class_loader_data();
if (!SystemDictionaryShared::is_sharing_possible(loader_data)) {
return;
}
bool skip = false;
if (is_shared()) {
assert(stream == NULL, "shared class with stream");
if (is_hidden()) {
// Don't include archived lambda proxy class in the classlist.
assert(!is_non_strong_hidden(), "unexpected non-strong hidden class");
return;
}
} else {
assert(stream != NULL, "non-shared class without stream");
// skip hidden class and unsafe anonymous class.
if ( is_hidden() || unsafe_anonymous_host() != NULL) {
return;
}
oop class_loader = loader_data->class_loader();
if (class_loader == NULL || SystemDictionary::is_platform_class_loader(class_loader)) {
// For the boot and platform class loaders, skip classes that are not found in the
// java runtime image, such as those found in the --patch-module entries.
// These classes can't be loaded from the archive during runtime.
if (!stream->from_boot_loader_modules_image() && strncmp(stream->source(), "jrt:", 4) != 0) {
skip = true;
if (is_shareable()) {
bool skip = false;
if (is_shared()) {
if (stream == nullptr) {
tty->print_cr("%s: shared class with stream?", name()->as_C_string());
return; // stream is nullptr will cause below operation fail.
}
} else {
if (stream != nullptr) {
oop class_loader = class_loader_data()->class_loader();
if (class_loader == NULL || SystemDictionary::is_platform_class_loader(class_loader)) {
// For the boot and platform class loaders, skip classes that are not found in the
// java runtime image, such as those found in the --patch-module entries.
// These classes can't be loaded from the archive during runtime.
if (!stream->from_boot_loader_modules_image() && strncmp(stream->source(), "jrt:", 4) != 0) {
skip = true;
}

if (class_loader == NULL && ClassLoader::contains_append_entry(stream->source())) {
This conversation was marked as resolved by yminqi

This comment has been minimized.

@calvinccheung

calvinccheung Mar 24, 2021
Member

Since the above has been removed, I think the ClassLoader::contains_append_entry() function can be removed too.

// .. but don't skip the boot classes that are loaded from -Xbootclasspath/a
// as they can be loaded from the archive during runtime.
skip = false;
if (class_loader == NULL && ClassLoader::contains_append_entry(stream->source())) {
// .. but don't skip the boot classes that are loaded from -Xbootclasspath/a
// as they can be loaded from the archive during runtime.
skip = false;
}
}
} else {
warning("non-shared class without stream, skipped");
return;
}
}
}
ResourceMark rm;
if (skip) {
tty->print_cr("skip writing class %s from source %s to classlist file",
name()->as_C_string(), stream->source());
} else {
ClassListWriter w;
w.stream()->print_cr("%s", name()->as_C_string());
w.stream()->flush();
if (skip) {
tty->print_cr("skip writing class %s from source %s to classlist file",
name()->as_C_string(), stream->source());
} else {
ClassListWriter w;
w.stream()->print_cr("%s", name()->as_C_string());
w.stream()->flush();
}
}
}
#endif // INCLUDE_CDS
@@ -3763,15 +3763,15 @@ JVM_END

JVM_ENTRY(void, JVM_DumpDynamicArchive(JNIEnv *env, jstring archiveName))
#if INCLUDE_CDS
assert(UseSharedSpaces && RecordDynamicDumpInfo, "Sanity check");
assert(UseSharedSpaces && RecordDynamicDumpInfo, "already checked in arguments.cpp?");
if (DynamicArchive::has_been_dumped_once()) {

This comment has been minimized.

@iklam

iklam Mar 31, 2021
Member

Maybe add a comment like this:?

// During dynamic archive dumping, some of the data structures are overwritten so
// we cannot dump the dynamic archive again. TODO: this should be fixed.
THROW_MSG(vmSymbols::java_lang_RuntimeException(),
"Dynamic dump has been done, and should only be done once");
} else {
// prevent multiple dumps.
DynamicArchive::set_has_been_dumped_once();
}
assert(ArchiveClassesAtExit == nullptr, "Sanity check");
assert(ArchiveClassesAtExit == nullptr, "already checked in arguments.cpp?");
Handle file_handle(THREAD, JNIHandles::resolve_non_null(archiveName));
char* archive_name = java_lang_String::as_utf8_string(file_handle());
This conversation was marked as resolved by yminqi

This comment has been minimized.

@iklam

iklam Mar 31, 2021
Member

A ResourceMark is needed before calling java_lang_String::as_utf8_string().

In general, I think the code in jvm.cpp should only marshall the jobject argument (e.g., convert jstring to char*.). The main functionality of JVM_DumpDynamicArchive should be moved to dynamicArchive.cpp. Similarly, most of the work in JVM_DumpClassListToFile should be moved to metaspaceShared.cpp.

ArchiveClassesAtExit = archive_name;
@@ -1126,19 +1126,11 @@ void DumpSharedArchiveDCmd::execute(DCmdSource source, TRAPS) {
JavaCallArguments args;
args.push_int(is_static);
args.push_oop(fileh);
output()->print_cr("Call CDS.dumpSharedArchive(%s, %s)", scmd, (file == NULL ? "null" : file));
JavaCalls::call_static(&result,
cds_klass,
vmSymbols::dumpSharedArchive(),
vmSymbols::dumpSharedArchive_signature(),
&args, THREAD);

This comment has been minimized.

@iklam

iklam Apr 1, 2021
Member

Should be &args, CHECK);. Recently, we have started to to avoid using THREAD if the callee function may throw an exception.

// Upon exception, show stack trace.
if (HAS_PENDING_EXCEPTION) {
Handle throwable(THREAD, PENDING_EXCEPTION);
CLEAR_PENDING_EXCEPTION;
java_lang_Throwable::print_stack_trace(throwable, output());
output()->cr();
}
}

int DumpSharedArchiveDCmd::num_arguments() {
@@ -28,6 +28,7 @@
import java.io.File;
import java.io.InputStream;
import java.io.IOException;
import java.io.PrintStream;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.Map;
@@ -207,13 +208,15 @@ private static void validateInputLines(String[] lines) {
private static native void dumpClassList(String listFileName);
private static native void dumpDynamicArchive(String archiveFileName);

private static void outputStdStream(InputStream stream) {
private static void outputStdStream(InputStream stream, String fileName) {
String line;
InputStreamReader isr = new InputStreamReader(stream);
BufferedReader rdr = new BufferedReader(isr);

try {
PrintStream prt = new PrintStream(fileName);
while((line = rdr.readLine()) != null) {
System.out.println(line);
prt.println(line);
}
} catch (IOException e) {
throw new RuntimeException("IOExeption happens during drain stream " + e.getMessage());
@@ -246,9 +249,9 @@ private static boolean containsExcludedFlags(String testStr) {
* @param fileName user input archive name, can be null.
*/
private static void dumpSharedArchive(boolean isStatic, String fileName) throws Exception {
String currentPid = String.valueOf(ProcessHandle.current().pid());
String archiveFile = fileName != null ? fileName :
"java_pid" + ProcessHandle.current().pid() + (isStatic ? "_static.jsa" : "_dynamic.jsa");
System.out.println((isStatic ? "Static" : " Dynamic") + " dump to file " + archiveFile);
"java_pid" + currentPid + (isStatic ? "_static.jsa" : "_dynamic.jsa");

// delete if archive file aready exists
File fileArchive = new File(archiveFile);
@@ -290,26 +293,40 @@ private static void dumpSharedArchive(boolean isStatic, String fileName) throws
Process proc = Runtime.getRuntime().exec(cmds.toArray(new String[0]),
This conversation was marked as resolved by yminqi

This comment has been minimized.

@iklam

iklam Mar 10, 2021
Member

Could you explain why the parent's env variables will cause dumping to fail?

This comment has been minimized.

@yminqi

yminqi Mar 18, 2021
Author Contributor

I found jtreg env will be brought in to the children env which is not needed in this case. Add comment.

new String[] {"EnvP=null"});

// Drain stdout in a separate thread.
// Drain stdout to a file in a separate thread.
String stdOutFile = "java_pid" + proc.pid() + "_stdout.txt";
new Thread( ()-> {
System.out.println("Dumping process " + proc.pid() + " Stdout: ");
outputStdStream(proc.getInputStream());
outputStdStream(proc.getInputStream(), stdOutFile);
}).start();

// Drain stderr in a separate thread.
// Drain stderr to a file in a separate thread.
String stdErrFile = "java_pid" + proc.pid() + "_stderr.txt";
new Thread( ()-> {
System.out.println("Dumping process " + proc.pid() + " Stdout: ");
outputStdStream(proc.getErrorStream());
outputStdStream(proc.getErrorStream(), stdErrFile);
}).start();

This conversation was marked as resolved by yminqi

This comment has been minimized.

@iklam

iklam Apr 1, 2021
Member

I think the above code can be refactored to avoid duplication. Something like:

String stdOutFile = drainOutput(proc.getInputStream(), "stdout");
String stdErrFile = drainOutput(proc.getErrorStream(), "stderr");

and move the common code into drainOutput().

proc.waitFor();
// done, delete classlist file.
if (fileList.exists()) {
fileList.delete();
}
// Check if archive has been successfully dumped. We won't reach here if exception happens.
// Throw exception if file is not created.
if (!fileArchive.exists()) {
throw new RuntimeException("Archive file " + archiveFile +
" is not created, please check stdout file " +
stdOutFile + " or stderr file " +
stdErrFile + " for more detail");
}
} else {
dumpDynamicArchive(archiveFile);
if (!fileArchive.exists()) {
throw new RuntimeException("Archive file " + archiveFile +
" is not created, please check process " +
currentPid + " output for more detail");
}
}
// Check if archive has been successfully dumped. We won't reach here if exception happens.
// Throw exception if file is not created.
if (!fileArchive.exists()) {
throw new RuntimeException("Archive file " + archiveFile + " is not created");
}
// Everyting goes well, print out the file name.
System.out.println((isStatic ? "Static" : " Dynamic") + " dump to file " + archiveFile);
}
This conversation was marked as resolved by yminqi

This comment has been minimized.

@iklam

iklam Mar 10, 2021
Member

I think we should have some error checks and clean up:

  • Remove the classlist file
  • Check if if the process exit status is 0
  • Remove the JSA file first, then try to dump it, and check if the file exists afterwards. If not, report the error. (For both dynamic and static dumps)

This comment has been minimized.

@iklam

iklam Mar 19, 2021
Member

The classlist file is not deleted after the dump has finished.

}
@@ -324,6 +324,7 @@ hotspot_appcds_dynamic = \
-runtime/cds/appcds/javaldr/GCSharedStringsDuringDump.java \
-runtime/cds/appcds/javaldr/HumongousDuringDump.java \
-runtime/cds/appcds/javaldr/LockDuringDump.java \
-runtime/cds/appcds/jcmd/JCmdTest.java \
-runtime/cds/appcds/methodHandles \
-runtime/cds/appcds/sharedStrings \
-runtime/cds/appcds/ArchiveRelocationTest.java \
@@ -192,11 +192,11 @@ private static void test() throws Exception {
}
app.stopApp();

// 3. Test static dump with flags which no dump will happen.
// 3. Test static dump with flags with which dumping should fail
// This test will result classes.jsa in default server dir if -XX:SharedArchiveFile= not set.
print2ln("3. Test static dump with flags which will no dump will happen.");
print2ln("3. Test static dump with flags with which dumping should fail.");
for (String flag : noDumpFlags) {
app = createLingeredApp("-cp", jarFile, flag, "-XX:SharedArchiveFile=tmp.jsa");
app = createLingeredApp("-cp", jarFile, flag, "-XX:SharedArchiveFile=tmp.jsa");
// Following should not be executed.
if (app != null && app.getProcess().isAlive()) {
pid = app.getPid();
@@ -210,7 +210,7 @@ private static void test() throws Exception {
// 4. Test static dump with flags which will be filtered before dumping.
print2ln("4. Test static dump with flags which will be filtered before dumping.");
for (String flag : excludeFlags) {
app = createLingeredApp("-cp", jarFile, flag);
app = createLingeredApp("-cp", jarFile, flag);
pid = app.getPid();
test(SUBCMD_STATIC_DUMP, null, pid, EXPECT_PASS);
app.stopApp();
@@ -219,37 +219,58 @@ private static void test() throws Exception {

// 5. Test static with -Xshare:off will be OK to dump.
print2ln("5. Test static with -Xshare:off will be OK to dump.");
app = createLingeredApp("-Xshare:off", "-cp", jarFile);
app = createLingeredApp("-Xshare:off", "-cp", jarFile);
pid = app.getPid();
test(SUBCMD_STATIC_DUMP, null, pid, EXPECT_PASS);
app.stopApp();

// 6. Test dynamic dump with -XX:+RecordDynamicDumpInfo.
print2ln("6. Test dynamic dump with -XX:+RecordDynamicDumpInfo.");
app = createLingeredApp("-cp", jarFile, "-XX:+RecordDynamicDumpInfo");
// 6. Test static with --limit-modules java.base.
print2ln("6. Test static with --limit-modules java.base.");
app = createLingeredApp("--limit-modules", "java.base", "-cp", jarFile);
pid = app.getPid();
test(SUBCMD_STATIC_DUMP, null, pid, EXPECT_FAIL);

// 7. Test dynamic dump with -XX:+RecordDynamicDumpInfo.
print2ln("7. Test dynamic dump with -XX:+RecordDynamicDumpInfo.");
app = createLingeredApp("-cp", jarFile, "-XX:+RecordDynamicDumpInfo");
pid = app.getPid();
test(SUBCMD_DYNAMIC_DUMP, DYNAMIC_DUMP_FILE + "01", pid, EXPECT_PASS);

// 7. Test dynamic dump twice to same process.
print2ln("7. Test dynamic dump second time to the same process.");
// 8. Test dynamic dump twice to same process.
print2ln("8. Test dynamic dump second time to the same process.");
test(SUBCMD_DYNAMIC_DUMP, DYNAMIC_DUMP_FILE + "02", pid, EXPECT_FAIL);
app.stopApp();

// 8. Test dynamic dump with -XX:-RecordDynamicDumpInfo.
print2ln("8. Test dynamic dump with -XX:-RecordDynamicDumpInfo.");
app = createLingeredApp("-cp", jarFile);
// 9. Test dynamic dump with -XX:-RecordDynamicDumpInfo.
print2ln("9. Test dynamic dump with -XX:-RecordDynamicDumpInfo.");
app = createLingeredApp("-cp", jarFile);
pid = app.getPid();
test(SUBCMD_DYNAMIC_DUMP, DYNAMIC_DUMP_FILE + "01", pid, EXPECT_FAIL);
app.stopApp();

// 9. Test dynamic dump with default archive name (null).
print2ln("9. Test dynamic dump with default archive name (null).");
// 10. Test dynamic dump with default archive name (null).
print2ln("10. Test dynamic dump with default archive name (null).");
app = createLingeredApp("-cp", jarFile, "-XX:+RecordDynamicDumpInfo");
pid = app.getPid();
test(SUBCMD_DYNAMIC_DUMP, null, pid, EXPECT_PASS);
app.stopApp();

// 11. Test dynamic dump with flags -XX:+RecordDynamicDumpInfo -XX:-DynamicDumpSharedSpaces.
print2ln("11. Test dynamic dump with flags -XX:+RecordDynamicDumpInfo -XX:-DynamicDumpSharedSpaces.");
app = createLingeredApp("-cp", jarFile, "-XX:+RecordDynamicDumpInfo", "-XX:-DynamicDumpSharedSpaces");
pid = app.getPid();
test(SUBCMD_DYNAMIC_DUMP, null, pid, EXPECT_PASS);
app.stopApp();

// 12. Test dynamic dump with flags -XX:-DynamicDumpSharedSpaces -XX:+RecordDynamicDumpInfo.
print2ln("12. Test dynamic dump with flags -XX:-DynamicDumpSharedSpaces -XX:+RecordDynamicDumpInfo.");
app = createLingeredApp("-cp", jarFile, "-XX:-DynamicDumpSharedSpaces", "-XX:+RecordDynamicDumpInfo");
pid = app.getPid();
test(SUBCMD_DYNAMIC_DUMP, null, pid, EXPECT_PASS);
app.stopApp();

// 10. Test dynamic dump with -XX:ArchiveClassAtExit will fail.
print2ln("10. Test dynamic dump with -XX:ArchiveClassAtExit will fail.");
// 13. Test dynamic dump with -XX:ArchiveClassAtExit will fail.
print2ln("13. Test dynamic dump with -XX:ArchiveClassAtExit will fail.");
app = createLingeredApp("-cp", jarFile,
"-Xshare:auto",
"-XX:+RecordDynamicDumpInfo",
ProTip! Use n and p to navigate between commits in a pull request.