From 67fe357b71a262ac7d701ec64a3220baeaca8bb7 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Wed, 9 Mar 2022 13:35:39 -0500 Subject: [PATCH] Simplify Tracer.shouldObserve to avoid JIT friction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses a simple condition rather than a switch statement After: `javap -c -p ./tracing/build/classes/java/main/com/palantir/tracing/Tracer.class` ``` private static boolean shouldObserve(com.palantir.tracing.Observability); Code: 0: aload_0 1: getstatic #10 // Field com/palantir/tracing/Observability.SAMPLE:Lcom/palantir/tracing/Observability; 4: if_acmpeq 25 7: aload_0 8: getstatic #11 // Field com/palantir/tracing/Observability.UNDECIDED:Lcom/palantir/tracing/Observability; 11: if_acmpne 29 14: getstatic #12 // Field sampler:Lcom/palantir/tracing/TraceSampler; 17: invokeinterface #13, 1 // InterfaceMethod com/palantir/tracing/TraceSampler.sample:()Z 22: ifeq 29 25: iconst_1 26: goto 30 29: iconst_0 30: ireturn ``` Before: `javap -c -p ./tracing/build/classes/java/main/com/palantir/tracing/Tracer.class` ``` private static boolean shouldObserve(com.palantir.tracing.Observability); Code: 0: getstatic #10 // Field com/palantir/tracing/Tracer$1.$SwitchMap$com$palantir$tracing$Observability:[I 3: aload_0 4: invokevirtual #11 // Method com/palantir/tracing/Observability.ordinal:()I 7: iaload 8: tableswitch { // 1 to 3 1: 36 2: 38 3: 40 default: 49 } 36: iconst_1 37: ireturn 38: iconst_0 39: ireturn 40: getstatic #12 // Field sampler:Lcom/palantir/tracing/TraceSampler; 43: invokeinterface #13, 1 // InterfaceMethod com/palantir/tracing/TraceSampler.sample:()Z 48: ireturn 49: new #14 // class com/palantir/logsafe/exceptions/SafeIllegalArgumentException 52: dup 53: ldc #15 // String Unknown observability 55: iconst_1 56: anewarray #16 // class com/palantir/logsafe/Arg 59: dup 60: iconst_0 61: ldc #17 // String observability 63: aload_0 64: invokestatic #18 // Method com/palantir/logsafe/SafeArg.of:(Ljava/lang/String;Ljava/lang/Object;)Lcom/palantir/logsafe/SafeArg; 67: aastore 68: invokespecial #19 // Method com/palantir/logsafe/exceptions/SafeIllegalArgumentException."":(Ljava/lang/String;[Lcom/palantir/logsafe/Arg;)V 71: athrow ``` Benchmarks show improvement within variance: Benchmarks AFTER: ``` Benchmark (observability) Mode Cnt Score Error Units TracingBenchmark.traceWithSingleSpan SAMPLE avgt 3 533.719 ± 164.796 ns/op TracingBenchmark.traceWithSingleSpan DO_NOT_SAMPLE avgt 3 100.138 ± 35.819 ns/op TracingBenchmark.traceWithSingleSpan UNDECIDED avgt 3 122.807 ± 64.274 ns/op ``` Benchmarks BEFORE: ``` Benchmark (observability) Mode Cnt Score Error Units TracingBenchmark.traceWithSingleSpan SAMPLE avgt 3 522.865 ± 65.310 ns/op TracingBenchmark.traceWithSingleSpan DO_NOT_SAMPLE avgt 3 112.647 ± 48.826 ns/op TracingBenchmark.traceWithSingleSpan UNDECIDED avgt 3 118.643 ± 6.733 ns/op ``` --- .../src/main/java/com/palantir/tracing/Tracer.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index a54b26f66..840068c2e 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -26,7 +26,6 @@ import com.palantir.logsafe.Safe; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.UnsafeArg; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import com.palantir.logsafe.exceptions.SafeIllegalStateException; import com.palantir.logsafe.exceptions.SafeRuntimeException; import com.palantir.logsafe.logger.SafeLogger; @@ -86,16 +85,8 @@ private static Trace createTrace( } private static boolean shouldObserve(Observability observability) { - switch (observability) { - case SAMPLE: - return true; - case DO_NOT_SAMPLE: - return false; - case UNDECIDED: - return sampler.sample(); - } - - throw new SafeIllegalArgumentException("Unknown observability", SafeArg.of("observability", observability)); + // Simplified implementation of 'switch(observability) {' for fast inlining (30 bytes) + return observability == Observability.SAMPLE || (observability == Observability.UNDECIDED && sampler.sample()); } /**