Hi Philippe,
This is a good use of StackWalker. getSource can simply return
StackFrame and avoid the creation of String[].
Stuart will give you better guidance related to RMI testing.
I see that test/jdk/sun/rmi/runtime/Log has a few RMI logging tests.
RMI tests are in tier3. You can run jdk_rmi test group to verify
this patch.
I notice that pre-1.4 RMI logging support. I wonder if this is
time to remove it.
Mandy
On 8/22/19 7:21 AM, Philippe Marschall wrote:
Hello
First time contributor here.
We have exception tracing enabled in production and see thousands of
exceptions in sun.rmi.runtime.Log#getSource() to get the caller class.
In my option this should use StackWalker introduced in Java 9. I could
find no corresponding bug in JBS.
I created a and uploaded webrev here [1] and also added the patch inline
below. As for the implementation I created a custom StackWalker to be
able to pass in the estimated number of frames that we traverse. For
variable naming conventions I tried to stick with other StackWalker
users I could find in the JDK. The stream formatting tries to follow the
LVTI style guidelines.
I have not written a test as the method is private. I ran the tier1
tests and they passed.
I have signed the OCA.
What are the next steps?
--- old/src/java.rmi/share/classes/sun/rmi/runtime/Log.java 2019-08-21
14:16:47.381544386 +0200
+++ new/src/java.rmi/share/classes/sun/rmi/runtime/Log.java 2019-08-21
14:16:47.249545585 +0200
@@ -30,6 +30,7 @@
import java.io.OutputStream;
import java.rmi.server.LogStream;
import java.security.PrivilegedAction;
+import java.util.Set;
import java.util.logging.Handler;
import java.util.logging.SimpleFormatter;
import java.util.logging.Level;
@@ -62,6 +63,8 @@
public static final Level BRIEF = Level.FINE;
public static final Level VERBOSE = Level.FINER;
+ private static final StackWalker WALKER =
StackWalker.getInstance(Set.of(), 4);
+
/* selects log implementation */
private static final LogFactory logFactory;
static {
@@ -444,10 +447,10 @@
* Obtain class and method names of code calling a log method.
*/
private static String[] getSource() {
- StackTraceElement[] trace = (new Exception()).getStackTrace();
- return new String[] {
- trace[3].getClassName(),
- trace[3].getMethodName()
- };
+ return WALKER.walk(s -> s
+ .skip(3)
+ .map(f -> new String[]
{f.getClassName(), f.getMethodName()})
+ .findFirst()
+ .get());
}
}
[1]
https://github.com/marschall/webrevs/raw/master/Log-getSource/webrev.zip
Cheers
Philippe Marschall
