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

[Roger Riggs]

Hi Philippe,

The change looks good, I'll run it through our tests and push if its all ok.
(Pending anyone else's comments.)

On 8/23/19 10:38 AM, Philippe Marschall wrote:
>
>
> 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.
That's a start, running the complete suite of tests may catch something 
else or another dependency outside of the specific rmi tests.
>
>> 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.
Right, that's a separate issue and takes work and process to remove.
>
>
> I'll be away from a keyboard for a week so I won't be able to answer.

ok, Enjoy

Thanks, Roger

>
>
> --- 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