Performance: By using const over static, improve noop span creation performance another 10%

[shaun-cox]

Improves performance for span creation when the SDK is not configured another 10%.

(Depends on #1268 to go first.)

new_span/if_parent_sampled/no-sdk/spec
                        time:   [83.124 ns 83.224 ns 83.312 ns]
                        thrpt:  [12.003 Melem/s 12.016 Melem/s 12.030 Melem/s]
                 change:
                        time:   [-9.3988% -9.2273% -9.0668%] (p = 0.00 < 0.05)
                        thrpt:  [+9.9709% +10.165% +10.374%]
                        Performance has improved.

Changes

• remove branch in build_with_context for the noop tracer. the current context's span() method already returns &NOOP_SPAN in the case when no active span is present, which already has the right span context value to use.
• changed NOOP_SPAN from a lazy static to a const
• changed other constituents of span context to use const default values too.

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

Patch coverage is 100.0% of modified lines.

Files Changed	Coverage
opentelemetry/src/trace/context.rs	ø
opentelemetry/src/trace/noop.rs	100.0%
opentelemetry/src/trace/span_context.rs	100.0%

📢 Thoughts on this report? Let us know!.

[shaun-cox]

There are a lot of places using empty_context() that can be replaced with SpanContext::NONE. I haven't changed them in this PR, but my plan would be to change them all in a future PR and then remove the empty_context() function. Anyone see any problems with that approach?

[djc]

Nice!

[djc]

Style suggestion: I usually like to put consts near the bottom instead of near the top, since they're usually relatively simple and don't change often.

[lalitb]

Just to understand more, the performance improvement here is for the first call to NoopTracer::build_with_span(), when the static NOOP_SPAN (in existing code) is initialized. As the subsequent calls to NoopTracer::build_with_span() will utilize this already created NOOP_SPAN.

[shaun-cox]

Just to understand more, the performance improvement here is for the first call to NoopTracer::build_with_span(), when the static NOOP_SPAN (in existing code) is initialized. As the subsequent calls to NoopTracer::build_with_span() will utilize this already created NOOP_SPAN.

Good question. It's not so much about optimizing the first call which accesses a lazy static (to save the initialization), its more about optimizing every call to the lazy static which has to check that that initialization has been done before proceeding. When it's a constant, there is nothing to initialize, so there is no need for a call to check that it has been initialized.

The generated assembly for this sample code should make it clearer.

example::access_static:
        push    rax
        lea     rdi, [rip + example::STATIC_THING]
        call    qword ptr [rip + <once_cell::sync::Lazy<T,F> as core::ops::deref::Deref>::deref@GOTPCREL]
        pop     rcx
        ret

example::access_const:
        lea     rax, [rip + .L__unnamed_20]
        ret

[shaun-cox]

Oh, and I should have set the optimization flag for the compiler for release, above. When I do that, the results are even more obvious. :)

example::access_static:
        push    rax
        lea     rax, [rip + example::STATIC_THING]
        mov     qword ptr [rsp], rax
        movzx   eax, byte ptr [rip + example::STATIC_THING+24]
        cmp     al, 2
        jne     .LBB11_4
.LBB11_1:
        movzx   eax, byte ptr [rip + example::STATIC_THING+24]
        cmp     al, 2
        jne     .LBB11_7
        cmp     qword ptr [rip + example::STATIC_THING], 0
        je      .LBB11_3
        lea     rax, [rip + example::STATIC_THING+8]
        pop     rcx
        ret
.LBB11_4:
        mov     rdi, rsp
        call    once_cell::imp::OnceCell<T>::initialize
        movzx   eax, byte ptr [rip + example::STATIC_THING+24]
        cmp     al, 2
        je      .LBB11_1
        lea     rdi, [rip + .L__unnamed_11]
        lea     rdx, [rip + .L__unnamed_12]
        mov     esi, 41
        call    qword ptr [rip + core::panicking::panic@GOTPCREL]
        ud2
.LBB11_7:
        lea     rdi, [rip + .L__unnamed_13]
        lea     rdx, [rip + .L__unnamed_14]
        mov     esi, 39
        call    qword ptr [rip + core::panicking::panic@GOTPCREL]
        ud2
.LBB11_3:
        lea     rdi, [rip + .L__unnamed_5]
        lea     rdx, [rip + .L__unnamed_15]
        mov     esi, 23
        call    qword ptr [rip + core::panicking::panic@GOTPCREL]
        ud2

example::STATIC_THING:
        .zero   8
        .zero   16
        .zero   1
        .zero   7
        .quad   example::Thing::new

vs

example::access_const:
        lea     rax, [rip + .L__unnamed_16]
        ret

.L__unnamed_16:
        .zero   16

[lalitb]

The generated assembly for this sample code should make it clearer.

Thanks, the assembly makes it much clear.