Skip to content

Commit

Permalink
[QuickStart] Remove strict argument verification logic
Browse files Browse the repository at this point in the history
Summary: QuickStart's JVM argument verification is too strict: there are only several options that need to be verified, like 'UseCompressedOops', 'UseTLAB', etc. So we want to loose it by delegating the logic to CDS and AOT themselves.

Test Plan: all quickstart tests

Reviewers: lei.yul, sanhong.lsh
  • Loading branch information
jia-wei-tang committed Jul 12, 2023
1 parent 23d8b2b commit d56de5d
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 116 deletions.
5 changes: 5 additions & 0 deletions src/hotspot/share/cds/metaspaceShared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
#include "runtime/arguments.hpp"
#include "runtime/handles.inline.hpp"
#include "runtime/os.hpp"
#include "runtime/quickStart.hpp"
#include "runtime/safepointVerifiers.hpp"
#include "runtime/sharedRuntime.hpp"
#include "runtime/vmThread.hpp"
Expand Down Expand Up @@ -1091,6 +1092,10 @@ MapArchiveResult MetaspaceShared::map_archives(FileMapInfo* static_mapinfo, File
// with the archived ones, so it must be done after all encodings are determined.
static_mapinfo->map_heap_regions();
}
if (QuickStart::is_enabled()) {
QuickStart::set_opt_passed(QuickStart::_appcds);
QuickStart::set_opt_passed(QuickStart::_eagerappcds);
}
});
log_info(cds)("optimized module handling: %s", MetaspaceShared::use_optimized_module_handling() ? "enabled" : "disabled");
log_info(cds)("full module graph: %s", MetaspaceShared::use_full_module_graph() ? "enabled" : "disabled");
Expand Down
144 changes: 37 additions & 107 deletions src/hotspot/share/runtime/quickStart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,13 @@ const char* QuickStart::_identifier_name[] = {
};

bool QuickStart::_opt_enabled[] = {
#define OPT_TAG(name) true,
OPT_TAG_LIST
#undef OPT_TAG
true, // appcds
true, // eagerappcds
};

bool QuickStart::_opt_passed[] = {
false,
false,
};

FILE* QuickStart::_metadata_file = NULL;
Expand Down Expand Up @@ -212,10 +216,11 @@ void QuickStart::post_process_arguments(JavaVMInitArgs* options_args) {
// Determine the role
if (!determine_tracer_or_replayer(options_args)) {
_role = Normal;
_is_enabled = false;
setenv_for_roles();
return;
}
// Set environment for roles of this process
setenv_for_roles();
settle_opt_pass_table();
// Process argument for each optimization
process_argument_for_optimization();
}
Expand Down Expand Up @@ -268,48 +273,6 @@ void QuickStart::check_features(const char* &str) {
}
}

static bool need_to_ignore(const char *option) {
// options need to be ignored
static const char *skip_list[] = { // use strncmp to compare
"-D", // skip -D java options
"-Xquickstart", // skip -Xquickstart* options

"-Xmx", // skip -Xmx related options
"-XX:InitialHeapSize",
"-XX:MaxHeapSize",
"-XX:InitialCodeCacheSize",
"-XX:ReservedCodeCacheSize",
"-XX:MaxMetaspaceSize",
"-XX:MetaspaceSize",
"-XX:MaxNewSize",
"-XX:NewSize",
"-Xms",
"-Xlog",
};

static const int size = sizeof(skip_list) / sizeof(const char*);
// traverse
for (int i = 0; i < size; i++) {
const char *skip = skip_list[i];
if (strncmp(option, skip, strlen(skip)) == 0) {
return true;
}
}
return false;
}

static int compare_strings(const char** s1, const char** s2) {
return ::strcmp(*s1, *s2);
}

static void free_tmp_buffers(GrowableArray<const char *> *&prev_options, GrowableArray<const char *> *&current_options) {
delete current_options; current_options = NULL;
for (int i = 0; i < prev_options->length(); i++) {
free((void*)prev_options->at(i));
}
delete prev_options; prev_options = NULL;
}

bool QuickStart::load_and_validate(JavaVMInitArgs* options_args) {
char line[PATH_MAX];
const char* tail = NULL;
Expand Down Expand Up @@ -358,72 +321,15 @@ bool QuickStart::load_and_validate(JavaVMInitArgs* options_args) {
return false;
}
option_checked = true;
// current jvm option count
int current_valid_option_count = 0;
if (options_args != NULL) {
int option_count = options_args->nOptions > 2 ? options_args->nOptions - 2 : 0;
if (_jvm_option_count != option_count) {
log("JVM option isn't the same.");
return false;
}
}
// record prev options with skipping ones ignored
// We delegate argument checking logic to CDS and AOT themselves.
// Just sanity check reading the file and ignore the results.
// Note: at this time of argument parsing, we cannot use Thread local and ResourceMark
GrowableArray<const char *> *prev_options = new (ResourceObj::C_HEAP, mtArguments) GrowableArray<const char *>(_jvm_option_count, mtInternal);
GrowableArray<const char *> *current_options = new (ResourceObj::C_HEAP, mtArguments) GrowableArray<const char *>(current_valid_option_count, mtInternal);
for (int index = 0; index < _jvm_option_count; index++) {
if (fgets(line, sizeof(line), _metadata_file) == NULL) {
log_error(quickstart)("Unable to read JVM option.");
free_tmp_buffers(prev_options, current_options);
return false;
}
if (options_args == NULL) {
continue;
}
// skip what we don't want
if (need_to_ignore(line)) {
continue;
}

prev_options->append(strdup(line));
}
// record current options
for (int index = 0; index < current_valid_option_count; index++) {
const JavaVMOption *option = options_args->options + index;
if (need_to_ignore(option->optionString)) {
continue;
}
current_options->append(option->optionString);
}
// 1. compare total numbers
int prev_valid_option_count = prev_options->length();
current_valid_option_count = current_options->length();
if (prev_valid_option_count != current_valid_option_count) {
log_error(quickstart)("JVM option count isn't the same.");
free_tmp_buffers(prev_options, current_options);
return false;
}
if (prev_valid_option_count == 0) {
free_tmp_buffers(prev_options, current_options);
return true;
}
// 2. sort all options
prev_options->sort(compare_strings);
current_options->sort(compare_strings);
// 3. check if option counts are equal?
for (int index = 0; index < current_valid_option_count; index++) {
size_t len = strlen(current_options->at(index));
if (len > O_BUFLEN) {
len = PATH_MAX - 1;
}
if (strncmp(prev_options->at(index), current_options->at(index), len) != 0) {
log_error(quickstart)("JVM option isn't the same.");
free_tmp_buffers(prev_options, current_options);
return false;
}
}

free_tmp_buffers(prev_options, current_options);
}
}
return true;
Expand Down Expand Up @@ -490,7 +396,6 @@ void QuickStart::print_stat(bool isReplayer) {
}

void QuickStart::setenv_for_roles() {
assert(is_enabled(), "sanity");
const char *role = NULL;
if (_role == Tracer) {
role = "TRACER";
Expand Down Expand Up @@ -622,6 +527,31 @@ bool QuickStart::determine_tracer_or_replayer(JavaVMInitArgs* options_args) {
return false;
}

void QuickStart::settle_opt_pass_table() {
// If a feature is disabled by quickstart, we have no need to check it
// So it is directly passed - so set it to true (already passed).
// If a feature is settled to be enabled by quickstart, then we need to
// check if it is successfully passed - so set it to false (not passed yet).
_opt_passed[_eagerappcds] = !_opt_enabled[_eagerappcds];
_opt_passed[_appcds] = !_opt_enabled[_appcds];
}

void QuickStart::set_opt_passed(opt feature) {
// set a feature passed
_opt_passed[feature] = true;
log_info(quickstart)("feature %s is enabled and passed", _opt_name[feature]);

// If all features are passed, we set the environment for roles of this process
bool opt_all_passed = true;
for (int i = 0; i < Count; i++) {
opt_all_passed &= _opt_passed[i];
}
if (opt_all_passed) {
log_info(quickstart)("all enabled features are passed");
setenv_for_roles();
}
}

void QuickStart::generate_metadata_file() {
// mv metadata to metadata.tmp
delete _temp_metadata_file;
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/runtime/quickStart.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class QuickStart : AllStatic {
static const char* _opt_name[];
static const char* _identifier_name[];
static bool _opt_enabled[];
static bool _opt_passed[];
static int _jvm_option_count;

static bool set_optimization(const char* option, bool enabled);
Expand All @@ -82,8 +83,10 @@ class QuickStart : AllStatic {
static void check_features(const char* &str);
static void print_stat(bool isReplayer);
static void log(const char* msg, ...) ATTRIBUTE_PRINTF(1, 2);
static void settle_opt_pass_table();

public:
static void set_opt_passed(opt feature);
static void notify_dump();

// cds stuff
Expand Down
136 changes: 136 additions & 0 deletions test/jdk/com/alibaba/quickstart/TestArgumentVerification.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import jdk.test.lib.Asserts;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;
import sun.security.action.GetPropertyAction;

import java.io.File;
import java.io.IOException;
import java.security.AccessController;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;


/*
* @test
* @modules java.base/sun.security.action
* @summary Test quick start dump when enable AOT.
* @library /test/lib
* @library /lib/testlibrary
* @requires os.arch=="amd64"
* @run main/othervm/timeout=1200 TestArgumentVerification
*/
public class TestArgumentVerification {

private static String workDir = AccessController.doPrivileged(new GetPropertyAction("test.classes"));
private static File cacheDir = createCacheDir(workDir);
private static ArrayList<String> postBasicVmOpts = new ArrayList<>();
static {
try {
// initialize basicVmOpts
Collections.addAll(postBasicVmOpts, new String[] {
"-Xquickstart:path=" + cacheDir.getCanonicalPath(),
"-Xquickstart:verbose," + Config.QUICKSTART_FEATURE,
"-XX:+UnlockDiagnosticVMOptions",
"-XX:+UseAOTStrictLoading",
"-version"
// no other arguments
});
} catch (IOException e) {
throw new Error(e);
}
}

public static class Pair {
public String[] tracerOpts;
public String[] replayerOpts;
public boolean shouldPass;

public Pair(String[] tracerOpts, String[] replayerOpts, boolean shouldPass) {
this.tracerOpts = tracerOpts;
this.replayerOpts = replayerOpts;
this.shouldPass = shouldPass;
}
}

private static final ArrayList<Pair> testOptsCombinations = new ArrayList<>();
static {
// test CompressedOops
testOptsCombinations.add(new Pair(
new String[] { "-XX:+UseCompressedOops" },
new String[] { "-XX:-UseCompressedOops" },
false
));
testOptsCombinations.add(new Pair(
new String[] { "-XX:-UseCompressedOops" },
new String[] { "-XX:+UseCompressedOops" },
false
));
// UseTLAB
testOptsCombinations.add(new Pair(
new String[] { "-XX:-UseTLAB" },
new String[] { "-XX:+UseTLAB" },
false
));
}

public static void main(String[] args) throws Exception {
test();
}

static void test() throws Exception {
for (int i = 0; i < testOptsCombinations.size(); i++) {
destroyCache(cacheDir);
runTracer(i);
runReplayer(i);
}
}

static void runTracer(int index) throws Exception {
ArrayList<String> vmOpts = new ArrayList<>();
vmOpts.addAll(Arrays.asList(testOptsCombinations.get(index).tracerOpts));
vmOpts.addAll(postBasicVmOpts);

ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(vmOpts.toArray(new String[vmOpts.size()]));
pb.redirectErrorStream(true);
OutputAnalyzer output = new OutputAnalyzer(pb.start());
System.out.println("[Tracer Output] " + output.getOutput());
output.shouldContain(TestDump.ANCHOR);
output.shouldHaveExitValue(0);
}

static void runReplayer(int index) throws Exception {
ArrayList<String> vmOpts = new ArrayList<>();
vmOpts.addAll(Arrays.asList(testOptsCombinations.get(index).replayerOpts));
vmOpts.addAll(postBasicVmOpts);

boolean shouldPass = testOptsCombinations.get(index).shouldPass;

ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(vmOpts.toArray(new String[vmOpts.size()]));
pb.redirectErrorStream(true);
OutputAnalyzer output = new OutputAnalyzer(pb.start());
System.out.println("[Replayer Output] " + output.getOutput());
if (shouldPass) {
output.shouldHaveExitValue(0);
} else {
Asserts.assertTrue(output.getExitValue() != 0);
}
}

private static File createCacheDir(String workDir) {
File cacheDir = new File(workDir, "sharedCache");
if (!cacheDir.exists()) {
cacheDir.mkdirs();
}
return cacheDir;
}

static void destroyCache(File cacheDir) throws Exception {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-Xquickstart:destroy", "-Xquickstart:path=" + cacheDir.getCanonicalPath(), "-Xquickstart:verbose", "-version");
OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.shouldContain("destroy the cache folder");
output.shouldHaveExitValue(0);
}


}
Loading

0 comments on commit d56de5d

Please sign in to comment.