Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excavator: Upgrades Baseline to the latest version #19

Merged
merged 3 commits into from
Oct 29, 2018

Conversation

svc-excavator-bot
Copy link
Collaborator

excavator is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

{runtimeCheckDesc}
To enable or disable this check, please contact the maintainers of Excavator.

@svc-excavator-bot svc-excavator-bot requested a review from a team as a code owner October 11, 2018 10:25
@svc-excavator-bot svc-excavator-bot force-pushed the roomba/latest-baseline-oss branch 4 times, most recently from df90d76 to 0b0268b Compare October 17, 2018 15:05
@svc-excavator-bot svc-excavator-bot force-pushed the roomba/latest-baseline-oss branch from 0b0268b to 90c7238 Compare October 18, 2018 03:40
@iamdanfox iamdanfox force-pushed the roomba/latest-baseline-oss branch from 59653e6 to bb96162 Compare October 18, 2018 13:21
@iamdanfox iamdanfox merged commit 8087a9b into develop Oct 29, 2018
@iamdanfox iamdanfox deleted the roomba/latest-baseline-oss branch October 29, 2018 11:53
carterkozak added a commit that referenced this pull request Mar 9, 2022
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
```
carterkozak added a commit that referenced this pull request Mar 9, 2022
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
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants