Remove exception from sun.rmi.runtime.Log#getSource()

[Philippe Marschall]

On 22.08.19 19:56, Mandy Chung wrote:
> Hi Philippe,
>
> This is a good use of StackWalker.   getSource can simply return
> StackFrame and avoid the creation of String[].

Updated webrev [1] and inline diff at the end.

Something I noted is that we could replace
sun.rmi.runtime.Log.LogStreamLog.unqualifiedName(String) with
Class#getSimpleName() which is cached since JDK-8187123. That would
remove an additional bit of allocation. However only in the pre-1.4
logging case, which I assume is uncommon. In addition that would require
us to create a StackWalker with Option#RETAIN_CLASS_REFERENCE. If we do
not want to do this in the jul case we would have to create a LogFactory
specific #getSource implementation with a different StackWalker. I am
not sure this is worth it.

> 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 am new to this, I did

make run-test TEST=jdk_rmi

and all tests passed.

> I notice that pre-1.4 RMI logging support.  I wonder if this is
> time to remove it.

I would love to do this but I would assume that requires at least a
different issue and probably a change note that the property and feature
are gone. I assume deprecating java.rmi.server.LogStream for removal is
not an option.


I'll be away from a keyboard for a week so I won't be able to answer.


--- old/src/java.rmi/share/classes/sun/rmi/runtime/Log.java	2019-08-23
15:43:28.452810308 +0200
+++ new/src/java.rmi/share/classes/sun/rmi/runtime/Log.java	2019-08-23
15:43:28.320811517 +0200
@@ -28,8 +28,10 @@
  import java.io.ByteArrayOutputStream;
  import java.io.PrintStream;
  import java.io.OutputStream;
+import java.lang.StackWalker.StackFrame;
  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 +64,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 {
@@ -217,16 +221,16 @@

          public void log(Level level, String message) {
              if (isLoggable(level)) {
-                String[] source = getSource();
-                logger.logp(level, source[0], source[1],
+                StackFrame sourceFrame = getSource();
+                logger.logp(level, sourceFrame.getClassName(),
sourceFrame.getMethodName(),
                             Thread.currentThread().getName() + ": " +
message);
              }
          }

          public void log(Level level, String message, Throwable thrown) {
              if (isLoggable(level)) {
-                String[] source = getSource();
-                logger.logp(level, source[0], source[1],
+                StackFrame sourceFrame = getSource();
+                logger.logp(level, sourceFrame.getClassName(),
sourceFrame.getMethodName(),
                      Thread.currentThread().getName() + ": " +
                             message, thrown);
              }
@@ -390,9 +394,9 @@

          public void log(Level messageLevel, String message) {
              if (isLoggable(messageLevel)) {
-                String[] source = getSource();
-                stream.println(unqualifiedName(source[0]) +
-                               "." + source[1] + ": " + message);
+                StackFrame sourceFrame = getSource();
+
stream.println(unqualifiedName(sourceFrame.getClassName()) +
+                               "." + sourceFrame.getMethodName() + ": "
+ message);
              }
          }

@@ -403,9 +407,9 @@
                   * RemoteServer.getLog
                   */
                  synchronized (stream) {
-                    String[] source = getSource();
-                    stream.println(unqualifiedName(source[0]) + "." +
-                                   source[1] + ": " + message);
+                    StackFrame sourceFrame = getSource();
+
stream.println(unqualifiedName(sourceFrame.getClassName()) + "." +
+                                    sourceFrame.getMethodName() + ": "
+ message);
                      thrown.printStackTrace(stream);
                  }
              }
@@ -441,13 +445,12 @@
      }

      /**
-     * Obtain class and method names of code calling a log method.
+     * Obtain stack frame of code calling a log method.
       */
-    private static String[] getSource() {
-        StackTraceElement[] trace = (new Exception()).getStackTrace();
-        return new String[] {
-            trace[3].getClassName(),
-            trace[3].getMethodName()
-        };
+    private static StackFrame getSource() {
+        return WALKER.walk(s -> s
+                                 .skip(3)
+                                 .findFirst()
+                                 .get());
      }
  }

  [1]
https://github.com/marschall/webrevs/raw/master/Log-getSource-02/webrev.zip

Philippe