On Wed, 26 Nov 2025 21:38:59 GMT, Artur Barashev <[email protected]> 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
