RFR: 8371721: Refactor checkTrusted methods in X509TrustManagerImpl [v2]

[Sean Coffey]

On Wed, 26 Nov 2025 21:38:59 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java line 101:
>> 
>>> 99:         serverValidator = v;
>>> 100: 
>>> 101:         if (SSLLogger.isOn() && SSLLogger.isOn("ssl,trustmanager")) {
>> 
>> One benefit of the older style log call was that we got c2 inlining and some deadcode elimination in various security methods if the SSLLogger was off.  i.e. if `SSLLogger.isOn()` was proven to always be false, then c2 could remove such code paths.  The new `logFine` type calls bring one extra level of method reference but it might be worth confirming that c2 is able to inline such calls and perform dead code elimination also.
>
> I see, how do I confirm that then? I followed the example of `logWarning` method which was added earlier. It's more convenient way to log than current `if` condition approach.

I guess you've a few options to measure. One is `hsdis` tool. An easier approach is a jmh benchmark. I put a simple example together below. I think the performance will be similar/on-par with this design (tested with TLS logging disabled) but it does highlight a possible pitfall for future iterations of this code. The String and params passed to the new method are calculated even before checking to see if logging is enabled. For an expensive string calculation, that'll add cycles. see `testLogProposed` Benchmark. A Supplier type API might be useful in future versions.


Benchmark                                  (numStrings)  Mode  Cnt    Score    Error  Units
SSLLoggerTest.testLogCurrent                          1  avgt    5    0.391 ±  0.027  ns/op
SSLLoggerTest.testLogCurrent                         10  avgt    5    0.391 ±  0.021  ns/op
SSLLoggerTest.testLogProposed                         1  avgt    5   64.012 ±  8.939  ns/op
SSLLoggerTest.testLogProposed                        10  avgt    5  242.656 ± 22.269  ns/op
SSLLoggerTest.testLogProposedSimpleString             1  avgt    5    0.386 ±  0.016  ns/op
SSLLoggerTest.testLogProposedSimpleString            10  avgt    5    0.389 ±  0.021  ns/op
Finished running test 'micro:sun.security.ssl.SSLLoggerTest'


JMH src:


package org.openjdk.bench.sun.security.ssl;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;

import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import sun.security.ssl.SSLLogger;

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Thread)
@Fork(value = 1, jvmArgs = {"--add-exports", "java.base/sun.security.ssl=ALL-UNNAMED"})
public class SSLLoggerTest {

    @Param({"1", "10"})
    private int numStrings;

    @Benchmark
    public void testLogCurrent() {
        if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
            SSLLogger.info(createMessage(numStrings));
        }
    }

    @Benchmark
    public void testLogProposed() {
        SSLLogger.logFine("ssl,handshake", createMessage(numStrings));
    }

    @Benchmark
    public void testLogProposedSimpleString() {
        SSLLogger.logFine("ssl,handshake", "Test String Logging Call");
    }

    private static String createMessage(int numStrings) {
        return IntStream.range(0, numStrings).mapToObj(i -> { return "test" + i; }).collect(Collectors.toList()).toString();
    }
}

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28275#discussion_r2576686296