Skip to content

Commit

Permalink
Simplify Tracer.shouldObserve to avoid JIT friction
Browse files Browse the repository at this point in the history
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."<init>":(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
```
  • Loading branch information
carterkozak committed Mar 9, 2022
1 parent a998686 commit 67fe357
Showing 1 changed file with 2 additions and 11 deletions.
13 changes: 2 additions & 11 deletions tracing/src/main/java/com/palantir/tracing/Tracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

/**
Expand Down

0 comments on commit 67fe357

Please sign in to comment.