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

8334305: Remove all code for nsk.share.Log verbose mode #21267

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2024, 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 @@ -85,7 +85,7 @@ public static void main(String[] args) {
}

public int run() {
log = new Log(System.out, verbose);
log = new Log(System.out);
log.display("Parallel matrix multiplication test");

Matrix a = Matrix.randomMatrix(dim);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ private launchnosuspend001 (String args[], PrintStream out) {

argHandler = new ArgumentHandler(args);
log = new Log(this.out, argHandler);
//log.enableVerbose(true);
}

private PrintStream out;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2024, 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 @@ -49,7 +49,6 @@ public int runTest(String args[], PrintStream out) {
log = new Log(out, argHandler);
testObjects = new Object[]{new TaggedClass(),
new UntaggedClass()};
log.enableVerbose(true);
log.display("Verifying reachable objects.");
status = checkStatus(status);
testObjects = null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2004, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2004, 2024, 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 @@ -40,7 +40,6 @@ public static void main(String[] argv) {
public static int run(String[] argv, PrintStream out) {
ArgumentHandler argHandler = new ArgumentHandler(argv);
Log log = new Log(out, argHandler);
log.enableVerbose(true);

monitor = Monitor.getMemoryMonitor(log, argHandler);
List pools = monitor.getMemoryPoolMBeans();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2004, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2004, 2024, 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 @@ -47,7 +47,6 @@ public static void main(String[] argv) {
public static int run(String[] argv, PrintStream out) {
ArgumentHandler argHandler = new ArgumentHandler(argv);
Log log = new Log(out, argHandler);
log.enableVerbose(true); // show log output

MemoryMonitor monitor = Monitor.getMemoryMonitor(log, argHandler);
List pools = monitor.getMemoryPoolMBeans();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2024, 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 @@ -58,7 +58,7 @@ public static void main(String[] args) {

@Override
public void run() {
Log log = new Log(System.out, true);
Log log = new Log(System.out);
// System.err is duplicated into buffer
// it should be empty
MyStream stream = new MyStream(System.err);
Expand Down
118 changes: 12 additions & 106 deletions test/hotspot/jtreg/vmTestbase/nsk/share/Log.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,15 @@
import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringReader;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.HashSet;
import java.util.Vector;

import nsk.share.test.LazyFormatString;

/**
* This class helps to print test-execution trace messages
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing period.

* and filter them when execution mode is not verbose.
* <p>
* Verbose mode if defined by providing <i>-verbose</i> command line
* option, handled by <code>ArgumentParser</code>. Use <code>verbose()</code>
* method to determine which mode is used.
* <p>
* <code>Log</code> provides with two main methods to print messages:
* <ul>
Expand All @@ -60,7 +53,6 @@
* To provide printing messages from different sources into one log
* with distinct prefixes use internal <code>Log.Logger</code> class.
*
* @see #verbose()
* @see #complain(String)
* @see #display(String)
* @see ArgumentParser
Expand All @@ -72,18 +64,6 @@ public class Log {
*/
private PrintStream out = null;

/**
* Is log-mode verbose?
* Always enabled.
*/
private final boolean verbose = true;

/**
* Should log messages prefixed with timestamps?
* Always enabled.
*/
private final boolean timestamp = true;

/**
* Names for trace levels
*/
Expand Down Expand Up @@ -188,41 +168,14 @@ public Log(PrintStream stream) {

/**
* Incarnate new Log for the given <code>stream</code>; and
* either for verbose or for non-verbose mode accordingly to
* the given <code>verbose</code> key.
*/
public Log(PrintStream stream, boolean verbose) {
this(stream);
}

/**
* Incarnate new Log for the given <code>stream</code>; and
* either for verbose or for non-verbose mode accordingly to
* the given <code>argsHandler</code>.
*/
public Log(PrintStream stream, ArgumentParser argsParser) {
this(stream, argsParser.verbose());
traceLevel = argsParser.getTraceLevel();
}

/////////////////////////////////////////////////////////////////

/**
* Return <i>true</i> if log mode is verbose.
*/
public boolean verbose() {
return verbose;
}

/**
* Enable or disable verbose mode for printing messages.
*/
public void enableVerbose(boolean enable) {
if (!enable) {
throw new RuntimeException("The non-verbose logging is not supported.");
}
}

public int getTraceLevel() {
return traceLevel;
}
Expand Down Expand Up @@ -266,9 +219,6 @@ public static String printExceptionToString(Object prefix, Throwable exception)
@Deprecated
public synchronized void println(String message) {
doPrint(message);
if (!verbose()) {
keepLog(composeLine(message));
}
}

/**
Expand All @@ -282,9 +232,6 @@ public synchronized void println(String message) {
*/
@Deprecated
public synchronized void comment(String message) {
if (!verbose()) {
doPrint(message);
}
Comment on lines -285 to -287
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method ever called? Is there a CR to remove it (and any references to it)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are deprecated methods and are still getting called. The calls need to be replaced with display. I will create a separate CR for this and address because the impact radius of such a change is bigger than this CR.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

}

/**
Expand Down Expand Up @@ -315,16 +262,9 @@ public void trace(int level, Object message, Throwable exception) {

/**
* Print <code>message</code> to the assigned output stream,
Copy link
Contributor

Choose a reason for hiding this comment

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

Need replace the comma with a period.

* if log mode is verbose. The <code>message</code> will be lost,
* if execution mode is non-verbose, and there is no error messages
* printed.
*/
public synchronized void display(Object message) {
if (verbose()) {
doPrint(message.toString());
} else {
keepLog(composeLine(message.toString()));
}
doPrint(message.toString());
}

/**
Expand All @@ -333,15 +273,6 @@ public synchronized void display(Object message) {
* into <code>errorsBuffer</code>.
*/
public synchronized void complain(Object message) {
if (!verbose()) {
PrintStream stream = findOutStream();
stream.println("#> ");
stream.println("#> WARNING: switching log to verbose mode,");
stream.println("#> because error is complained");
stream.println("#> ");
stream.flush();
enableVerbose(true);
}
String msgStr = message.toString();
printError(msgStr);

Expand Down Expand Up @@ -406,10 +337,9 @@ private void logExceptionForFailureAnalysis(String msg) {
/////////////////////////////////////////////////////////////////

/**
* Redirect log to the given <code>stream</code>, and switch
* log mode to verbose.
* Redirect log to the given <code>stream</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

Need period at end of sentence.

* Prints errors summary to current stream, cancel current stream
* and switches to new stream. Turns on verbose mode for new stream.
* and switches to new stream.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it really do all this? It looks to me like it just switches to the new stream. I'm not sure what is meant by "error summary" and cancelling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

marked for deprecation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, but unless it is going to be removed by another CR soon, I think the comments should at least reflect what it currently does.

*
* @deprecated This method is obsolete.
*/
Expand All @@ -430,20 +360,6 @@ public synchronized void clearLogBuffer() {
logBuffer.clear();
}

/**
* Print all messages from log buffer which were hidden because
* of non-verbose mode,
*/
private synchronized void flushLogBuffer() {
if (!logBuffer.isEmpty()) {
PrintStream stream = findOutStream();
for (int i = 0; i < logBuffer.size(); i++) {
stream.println(logBuffer.elementAt(i));
}
stream.flush();
}
}

/**
* Return <code>out</code> stream if defined or <code>Sytem.err<code> otherwise;
* print a warning message when <code>System.err</code> is used first time.
Expand All @@ -468,18 +384,15 @@ private synchronized PrintStream findOutStream() {
* Compose line to print possible prefixing it with timestamp.
*/
private String composeLine(String message) {
if (timestamp) {
long time = System.currentTimeMillis();
long ms = time % 1000;
time /= 1000;
long secs = time % 60;
time /= 60;
long mins = time % 60;
time /= 60;
long hours = time % 24;
return "[" + hours + ":" + mins + ":" + secs + "." + ms + "] " + message;
}
return message;
long time = System.currentTimeMillis();
long ms = time % 1000;
time /= 1000;
long secs = time % 60;
time /= 60;
long mins = time % 60;
time /= 60;
long hours = time % 24;
return "[" + hours + ":" + mins + ":" + secs + "." + ms + "] " + message;
}

/**
Expand Down Expand Up @@ -513,13 +426,6 @@ private synchronized void printError(String message) {
}
}

/**
* Keep the given log <code>message</code> into <code>logBuffer</code>.
*/
private synchronized void keepLog(String message) {
logBuffer.addElement(message);
}

/**
* This class can be used as a base for each class that use <code>Log</code>
* for print messages and errors.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2024, 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 @@ -68,7 +68,7 @@ public class AODTestRunner {
protected AODRunnerArgParser argParser;

protected AODTestRunner(String[] args) {
log = new Log(System.out, true);
log = new Log(System.out);

argParser = createArgParser(args);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2024, 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 @@ -157,7 +157,7 @@ private void defaultInit(String[] args) {
if (name == null)
throw new TestBug("Agent name wasn't specified");

log = new Log(System.out, true);
log = new Log(System.out);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
*/
public class DummyTargetApplication {

protected Log log = new Log(System.out, true);
protected Log log = new Log(System.out);

protected AODTargetArgParser argParser;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private void initTargetApplication(String[] args) {
if (targetApplicationInitialized)
throw new TestBug("TargetApplication already initialized");

log = new Log(System.out, true);
log = new Log(System.out);

argParser = createArgParser(args);

Expand Down
4 changes: 2 additions & 2 deletions test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/JVMTITest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2024, 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 @@ -87,7 +87,7 @@ static void attachAgent(String[] args) {

AgentsAttacher attacher = new AgentsAttacher(Utils.findCurrentVMIdUsingJPS(jdkPath),
agents,
new Log(System.out, true));
new Log(System.out));
attacher.attachAgents();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024, 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 @@ -69,7 +69,7 @@ public static void main(String[] args) throws Exception {

public void run() throws IOException, ReflectiveOperationException {

log = new Log(System.out, verbose);
log = new Log(System.out);

InstructionSequence instructionSequence = null;
for (int i = 0; i < iterations * stressOptions.getIterationsFactor(); i++) {
Expand Down