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

8256718: Obsolete the long term deprecated and aliased Trace flags #1525

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/hotspot/cpu/aarch64/methodHandles_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ void MethodHandles::jump_to_lambda_form(MacroAssembler* _masm,
assert(recv != noreg, "required register");
assert(method_temp == rmethod, "required register for loading method");

//NOT_PRODUCT({ FlagSetting fs(TraceMethodHandles, true); trace_method_handle(_masm, "LZMH"); });

// Load the invoker, as MH -> MH.form -> LF.vmentry
__ verify_oop(recv);
__ load_heap_oop(method_temp, Address(recv, NONZERO(java_lang_invoke_MethodHandle::form_offset())), temp2);
Expand Down
2 changes: 0 additions & 2 deletions src/hotspot/cpu/x86/methodHandles_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ void MethodHandles::jump_to_lambda_form(MacroAssembler* _masm,
assert(recv != noreg, "required register");
assert(method_temp == rbx, "required register for loading method");

//NOT_PRODUCT({ FlagSetting fs(TraceMethodHandles, true); trace_method_handle(_masm, "LZMH"); });

// Load the invoker, as MH -> MH.form -> LF.vmentry
__ verify_oop(recv);
__ load_heap_oop(method_temp, Address(recv, NONZERO(java_lang_invoke_MethodHandle::form_offset())), temp2);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/method.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ class Method : public Metadata {
// Printing
void print_short_name(outputStream* st = tty); // prints as klassname::methodname; Exposed so field engineers can debug VM
#if INCLUDE_JVMTI
void print_name(outputStream* st = tty); // prints as "virtual void foo(int)"; exposed for TraceRedefineClasses
void print_name(outputStream* st = tty); // prints as "virtual void foo(int)"; exposed for -Xlog:redefine+class
#else
void print_name(outputStream* st = tty) PRODUCT_RETURN; // prints as "virtual void foo(int)"
#endif
Expand Down
138 changes: 32 additions & 106 deletions src/hotspot/share/runtime/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,19 @@ static SpecialFlag const special_jvm_flags[] = {
{ "Debugging", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "UseRDPCForConstantTableBase", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "VerifyMergedCPBytecodes", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "PrintSharedSpaces", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceBiasedLocking", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceClassLoadingPreorder", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceClassPaths", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceClassResolution", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceInvokeDynamic", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceLoaderConstraints", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceMethodHandles", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceMonitorInflation", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceSafepointCleanupTime", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceJVMTIObjectTagging", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "TraceRedefineClasses", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
{ "PrintJNIResolving", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },

#ifdef TEST_VERIFY_SPECIAL_JVM_FLAGS
// These entries will generate build errors. Their purpose is to test the macros.
Expand Down Expand Up @@ -584,44 +597,14 @@ static AliasedFlag const aliased_jvm_flags[] = {
{ NULL, NULL}
};

// NOTE: A compatibility request will be necessary for each alias to be removed.
static AliasedLoggingFlag const aliased_logging_flags[] = {
{ "PrintSharedSpaces", LogLevel::Info, true, LOG_TAGS(cds) },
{ "TraceBiasedLocking", LogLevel::Info, true, LOG_TAGS(biasedlocking) },
{ "TraceClassLoading", LogLevel::Info, true, LOG_TAGS(class, load) },
{ "TraceClassLoadingPreorder", LogLevel::Debug, true, LOG_TAGS(class, preorder) },
{ "TraceClassPaths", LogLevel::Info, true, LOG_TAGS(class, path) },
{ "TraceClassResolution", LogLevel::Debug, true, LOG_TAGS(class, resolve) },
{ "TraceClassUnloading", LogLevel::Info, true, LOG_TAGS(class, unload) },
{ "TraceExceptions", LogLevel::Info, true, LOG_TAGS(exceptions) },
{ "TraceInvokeDynamic", LogLevel::Debug, true, LOG_TAGS(methodhandles, indy) },
{ "TraceLoaderConstraints", LogLevel::Info, true, LOG_TAGS(class, loader, constraints) },
{ "TraceMethodHandles", LogLevel::Info, true, LOG_TAGS(methodhandles) },
{ "TraceMonitorInflation", LogLevel::Trace, true, LOG_TAGS(monitorinflation) },
{ "TraceSafepointCleanupTime", LogLevel::Info, true, LOG_TAGS(safepoint, cleanup) },
{ "TraceJVMTIObjectTagging", LogLevel::Debug, true, LOG_TAGS(jvmti, objecttagging) },
{ "TraceRedefineClasses", LogLevel::Info, false, LOG_TAGS(redefine, class) },
{ "PrintJNIResolving", LogLevel::Debug, true, LOG_TAGS(jni, resolve) },
{ NULL, LogLevel::Off, false, LOG_TAGS(_NO_TAG) }
// Use this for popular obsolete tracing flags to suggest logging alternatives.
static AliasedObsoleteLoggingFlag const removed_product_logging_flags[] = {
{ "TraceClassLoading", "-Xlog:class+load=", "info", "16.0" },
{ "TraceClassUnloading", "-Xlog:class+unload=", "info", "16.0" },
{ "TraceExceptions", "-Xlog:exceptions=", "info", "16.0" },
{ NULL, NULL, NULL, NULL }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we wanted to give a message that the flag was obsolete and to suggest changing the command line, we should only do it for -XX:+TraceClassLoading and -XX:+TraceExceptions (I'd originally thought -XX:+TraceClassUnloading was important enough to release note but now I'm not so sure.) The rest of the flags should either go in the table that they're no longer recognized.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the message for any flag requires keeping all the supporting code. I don't see the "big 3" are special. They have been deprecated since 9 and we have clearly told people this when they use them. We're also release-noting this for 16 (again - this was documented when UL was added). I don't think we have to pander to anyone who hasn't updated their launch scripts by now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these three, I kind of like the pandering. I'm not sure that the release note will reach people using these, especially -XX:+TraceExceptions. I guess they've been getting a deprecation message since 9 so maybe it won't be such a surprise. I never stand in the way of removing code that people won't need, if you think this is the case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove them. There is a risk that the user might not notice the difference between the old and new messages and so not realize they don't actually have active logging any more (until they go to access the logs to diagnose a problem).

};

#ifndef PRODUCT
// These options are removed in jdk9. Remove this code for jdk10.
static AliasedFlag const removed_develop_logging_flags[] = {
{ "TraceClassInitialization", "-Xlog:class+init" },
{ "TraceClassLoaderData", "-Xlog:class+loader+data" },
{ "TraceDefaultMethods", "-Xlog:defaultmethods=debug" },
{ "TraceItables", "-Xlog:itables=debug" },
{ "TraceMonitorMismatch", "-Xlog:monitormismatch=info" },
{ "TraceSafepoint", "-Xlog:safepoint=debug" },
{ "TraceStartupTime", "-Xlog:startuptime" },
{ "TraceVMOperation", "-Xlog:vmoperation=debug" },
{ "PrintVtables", "-Xlog:vtables=debug" },
{ "VerboseVerification", "-Xlog:verification" },
{ NULL, NULL }
};
#endif //PRODUCT

// Return true if "v" is less than "other", where "other" may be "undefined".
static bool version_less_than(JDK_Version v, JDK_Version other) {
assert(!v.is_undefined(), "must be defined");
Expand Down Expand Up @@ -685,17 +668,15 @@ int Arguments::is_deprecated_flag(const char *flag_name, JDK_Version* version) {
return 0;
}

#ifndef PRODUCT
const char* Arguments::removed_develop_logging_flag_name(const char* name){
for (size_t i = 0; removed_develop_logging_flags[i].alias_name != NULL; i++) {
const AliasedFlag& flag = removed_develop_logging_flags[i];
if (strcmp(flag.alias_name, name) == 0) {
return flag.real_name;
const AliasedObsoleteLoggingFlag* Arguments::removed_product_logging_flag_name(const char* name){
for (size_t i = 0; removed_product_logging_flags[i].obs_name != NULL; i++) {
const AliasedObsoleteLoggingFlag* flag = &removed_product_logging_flags[i];
if (strcmp(flag->obs_name, name) == 0) {
return flag;
}
}
return NULL;
}
#endif // PRODUCT

const char* Arguments::real_flag_name(const char *flag_name) {
for (size_t i = 0; aliased_jvm_flags[i].alias_name != NULL; i++) {
Expand Down Expand Up @@ -1014,44 +995,6 @@ const char* Arguments::handle_aliases_and_deprecation(const char* arg, bool warn
return NULL;
}

void log_deprecated_flag(const char* name, bool on, AliasedLoggingFlag alf) {
LogTagType tagSet[] = {alf.tag0, alf.tag1, alf.tag2, alf.tag3, alf.tag4, alf.tag5};
// Set tagset string buffer at max size of 256, large enough for any alias tagset
const int max_tagset_size = 256;
int max_tagset_len = max_tagset_size - 1;
char tagset_buffer[max_tagset_size];
tagset_buffer[0] = '\0';

// Write tag-set for aliased logging option, in string list form
int max_tags = sizeof(tagSet)/sizeof(tagSet[0]);
for (int i = 0; i < max_tags && tagSet[i] != LogTag::__NO_TAG; i++) {
if (i > 0) {
strncat(tagset_buffer, "+", max_tagset_len - strlen(tagset_buffer));
}
strncat(tagset_buffer, LogTag::name(tagSet[i]), max_tagset_len - strlen(tagset_buffer));
}
if (!alf.exactMatch) {
strncat(tagset_buffer, "*", max_tagset_len - strlen(tagset_buffer));
}
log_warning(arguments)("-XX:%s%s is deprecated. Will use -Xlog:%s=%s instead.",
(on) ? "+" : "-",
name,
tagset_buffer,
(on) ? LogLevel::name(alf.level) : "off");
}

AliasedLoggingFlag Arguments::catch_logging_aliases(const char* name, bool on){
for (size_t i = 0; aliased_logging_flags[i].alias_name != NULL; i++) {
const AliasedLoggingFlag& alf = aliased_logging_flags[i];
if (strcmp(alf.alias_name, name) == 0) {
log_deprecated_flag(name, on, alf);
return alf;
}
}
AliasedLoggingFlag a = {NULL, LogLevel::Off, false, LOG_TAGS(_NO_TAG)};
return a;
}

bool Arguments::parse_argument(const char* arg, JVMFlagOrigin origin) {

// range of acceptable characters spelled out for portability reasons
Expand All @@ -1063,11 +1006,6 @@ bool Arguments::parse_argument(const char* arg, JVMFlagOrigin origin) {
bool warn_if_deprecated = true;

if (sscanf(arg, "-%" XSTR(BUFLEN) NAME_RANGE "%c", name, &dummy) == 1) {
AliasedLoggingFlag alf = catch_logging_aliases(name, false);
if (alf.alias_name != NULL){
LogConfiguration::configure_stdout(LogLevel::Off, alf.exactMatch, alf.tag0, alf.tag1, alf.tag2, alf.tag3, alf.tag4, alf.tag5);
return true;
}
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
Expand All @@ -1076,11 +1014,6 @@ bool Arguments::parse_argument(const char* arg, JVMFlagOrigin origin) {
return set_bool_flag(flag, false, origin);
}
if (sscanf(arg, "+%" XSTR(BUFLEN) NAME_RANGE "%c", name, &dummy) == 1) {
AliasedLoggingFlag alf = catch_logging_aliases(name, true);
if (alf.alias_name != NULL){
LogConfiguration::configure_stdout(alf.level, alf.exactMatch, alf.tag0, alf.tag1, alf.tag2, alf.tag3, alf.tag4, alf.tag5);
return true;
}
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
Expand All @@ -1094,11 +1027,6 @@ bool Arguments::parse_argument(const char* arg, JVMFlagOrigin origin) {
const char* value = strchr(arg, '=') + 1;

// this scanf pattern matches both strings (handled here) and numbers (handled later))
AliasedLoggingFlag alf = catch_logging_aliases(name, true);
if (alf.alias_name != NULL) {
LogConfiguration::configure_stdout(alf.level, alf.exactMatch, alf.tag0, alf.tag1, alf.tag2, alf.tag3, alf.tag4, alf.tag5);
return true;
}
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
Expand Down Expand Up @@ -1297,17 +1225,15 @@ bool Arguments::process_argument(const char* arg,
warning("Ignoring option %s; support was removed in %s", stripped_argname, version);
return true;
}
#ifndef PRODUCT
else {
const char* replacement;
if ((replacement = removed_develop_logging_flag_name(stripped_argname)) != NULL){
log_warning(arguments)("%s has been removed. Please use %s instead.",
stripped_argname,
replacement);
return false;
}
const AliasedObsoleteLoggingFlag* obs_replacement;
if (has_plus_minus && (obs_replacement = removed_product_logging_flag_name(stripped_argname)) != NULL) {
warning("Ignoring option %s; support was removed in %s. Please use %s%s instead.",
stripped_argname,
obs_replacement->version,
obs_replacement->log_name,
*arg == '+' ? obs_replacement->tag_name : "off");
return true;
}
hseigel marked this conversation as resolved.
Show resolved Hide resolved
#endif //PRODUCT
}

// For locked flags, report a custom error message if available.
Expand Down
21 changes: 6 additions & 15 deletions src/hotspot/share/runtime/arguments.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,18 +226,12 @@ class AgentLibraryList {
// Helper class for controlling the lifetime of JavaVMInitArgs objects.
class ScopedVMInitArgs;

// Most logging functions require 5 tags. Some of them may be _NO_TAG.
typedef struct {
const char* alias_name;
LogLevelType level;
bool exactMatch;
LogTagType tag0;
LogTagType tag1;
LogTagType tag2;
LogTagType tag3;
LogTagType tag4;
LogTagType tag5;
} AliasedLoggingFlag;
const char* obs_name;
const char* log_name;
const char* tag_name;
const char* version;
} AliasedObsoleteLoggingFlag;

class Arguments : AllStatic {
friend class VMStructs;
Expand Down Expand Up @@ -460,9 +454,7 @@ class Arguments : AllStatic {
// the version number when the flag became obsolete.
static bool is_obsolete_flag(const char* flag_name, JDK_Version* version);

#ifndef PRODUCT
static const char* removed_develop_logging_flag_name(const char* name);
#endif // PRODUCT
static const AliasedObsoleteLoggingFlag* removed_product_logging_flag_name(const char* name);

// Returns 1 if the flag is deprecated (and not yet obsolete or expired).
// In this case the 'version' buffer is filled in with the version number when
Expand All @@ -477,7 +469,6 @@ class Arguments : AllStatic {
// Return the "real" name for option arg if arg is an alias, and print a warning if arg is deprecated.
// Return NULL if the arg has expired.
static const char* handle_aliases_and_deprecation(const char* arg, bool warn);
static AliasedLoggingFlag catch_logging_aliases(const char* name, bool on);

static char* SharedArchivePath;
static char* SharedDynamicArchivePath;
Expand Down
3 changes: 0 additions & 3 deletions src/hotspot/share/runtime/globals.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -955,9 +955,6 @@ const intx ObjectAlignmentInBytes = 8;
\
/* JVMTI heap profiling */ \
\
product(bool, TraceJVMTIObjectTagging, false, DIAGNOSTIC, \
"Trace JVMTI object tagging calls") \
\
product(bool, VerifyBeforeIteration, false, DIAGNOSTIC, \
"Verify memory system before JVMTI iteration") \
\
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/*
* @test
* @bug 8048933
* @summary TraceExceptions output should have the exception message - useful for ClassNotFoundExceptions especially
* @summary -Xlog:exceptions=info output should have the exception message - useful for ClassNotFoundExceptions especially
* @library /test/lib
* @modules java.base/jdk.internal.misc
* java.management
Expand Down
10 changes: 0 additions & 10 deletions test/hotspot/jtreg/runtime/cds/appcds/ClassPathAttr.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,6 @@ static void testNormalOps() throws Exception {
output.shouldMatch("checking shared classpath entry: .*cpattr2.jar");
output.shouldMatch("checking shared classpath entry: .*cpattr3.jar");
});

// Make sure aliased TraceClassPaths still works
TestCommon.run(
"-XX:+TraceClassPaths",
"-cp", cp,
"CpAttr1")
.assertNormalExit(output -> {
output.shouldMatch("checking shared classpath entry: .*cpattr2.jar");
output.shouldMatch("checking shared classpath entry: .*cpattr3.jar");
});
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -46,13 +46,13 @@ public static void main(String[] args) throws Exception {
String cp_exec = sep + jar1 + sep + sep + jar2 + sep;

TestCommon.testDump(cp_dump, TestCommon.list("Hello", "HelloMore"),
"-XX:+TraceClassPaths", "-XX:+IgnoreEmptyClassPaths");
"-Xlog:class+path=info", "-XX:+IgnoreEmptyClassPaths");

TestCommon.run(
"-verbose:class",
"-cp", cp_exec,
"-XX:+IgnoreEmptyClassPaths", // should affect classpath even if placed after the "-cp" argument
"-XX:+TraceClassPaths",
"-Xlog:class+path=info",
"HelloMore")
.assertNormalExit();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static void main(String[] args) throws Throwable {
TestCommon.run(
"-cp", appJar,
"-Xmx32m",
"-XX:+PrintSharedSpaces",
"-Xlog:cds=info",
"-XX:+UnlockDiagnosticVMOptions", extraOption,
gcLog,
Hello.class.getName())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -112,7 +112,7 @@ public static void main(String[] args) throws Throwable {
extraArg,
"-Xlog:cds=info,class+path=info",
"-Xmx32m",
"-XX:+PrintSharedSpaces",
"-Xlog:cds=info",
"-XX:+UnlockDiagnosticVMOptions",
extraOption,
"-XX:+WhiteBoxAPI",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public static void main(String[] args) throws Throwable {
"-cp", appJar,
"-verbose",
"-Xmx64m",
"-XX:+PrintSharedSpaces",
"-Xlog:cds=info",
"-XX:+UnlockDiagnosticVMOptions", extraOption,
gcLog,
Hello.class.getName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public static void testBootAppendClassWithAppCDS() throws Exception {
OutputAnalyzer output = TestCommon.exec(
appJar,
"-Xbootclasspath/a:" + bootAppendJar,
"-XX:+TraceClassLoading",
"-Xlog:class+load=info",
MAIN_CLASS,
"Test #6", BOOT_APPEND_CLASS, "true", "BOOT");
TestCommon.checkExec(output);
Expand Down
Loading