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

Get rid of some bitwise checking in ddog_shall_log #2539

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented Feb 26, 2024

Description

Micro-optimization for logging

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi bwoebi force-pushed the bob/micro-optimize-shall_log branch from c77d8a1 to 8b3ba1b Compare February 26, 2024 18:52
@bwoebi bwoebi requested review from a team as code owners February 26, 2024 18:54
@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

Merging #2539 (c533eb6) into master (380774d) will increase coverage by 0.43%.
The diff coverage is 37.41%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2539      +/-   ##
============================================
+ Coverage     78.30%   78.73%   +0.43%     
  Complexity      267      267              
============================================
  Files           112      112              
  Lines         13458    13485      +27     
============================================
+ Hits          10538    10618      +80     
+ Misses         2920     2867      -53     
Flag Coverage Δ
tracer-extension 78.69% <37.41%> (+0.46%) ⬆️
tracer-integrations 79.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ext/integrations/integrations.c 97.88% <100.00%> (ø)
ext/priority_sampling/priority_sampling.c 95.02% <100.00%> (ø)
ext/request_hooks.c 93.24% <100.00%> (ø)
ext/handlers_curl.c 85.49% <0.00%> (ø)
ext/serializer.c 80.19% <66.66%> (ø)
ext/sidecar.h 62.50% <0.00%> (ø)
ext/startup_logging.c 89.37% <50.00%> (ø)
ext/comms_php.c 78.94% <33.33%> (ø)
ext/configuration.c 78.46% <0.00%> (ø)
ext/engine_hooks.c 85.18% <0.00%> (ø)
... and 9 more

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 380774d...c533eb6. Read the comment docs.

@pr-commenter
Copy link

pr-commenter bot commented Feb 26, 2024

Benchmarks

Benchmark execution time: 2024-02-27 15:57:05

Comparing candidate commit b436003 in PR branch bob/micro-optimize-shall_log with baseline commit 380774d in branch master.

Found 14 performance improvements and 1 performance regressions! Performance is the same for 167 metrics, 0 unstable metrics.

scenario:HookBench/benchHookOverheadInstallHookOnFunction

  • 🟩 execution_time [-198.808µs; -162.090µs] or [-19.801%; -16.144%]

scenario:HookBench/benchHookOverheadInstallHookOnFunction-opcache

  • 🟩 execution_time [-179.484µs; -154.377µs] or [-19.153%; -16.473%]

scenario:HookBench/benchHookOverheadInstallHookOnMethod

  • 🟩 execution_time [-204.271µs; -163.926µs] or [-20.286%; -16.279%]

scenario:HookBench/benchHookOverheadInstallHookOnMethod-opcache

  • 🟩 execution_time [-178.778µs; -158.802µs] or [-19.067%; -16.936%]

scenario:LogsInjectionBench/benchLogsInfoInjection

  • 🟩 execution_time [-441.399ns; -216.401ns] or [-4.950%; -2.427%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-5.446µs; -4.054µs] or [-3.691%; -2.748%]

scenario:PDOBench/benchPDOBaseline-opcache

  • 🟩 execution_time [-14.882µs; -14.113µs] or [-7.829%; -7.424%]

scenario:PDOBench/benchPDOOverhead-opcache

  • 🟩 execution_time [-19.437µs; -17.394µs] or [-6.459%; -5.780%]

scenario:PDOBench/benchPDOOverheadWithDBM-opcache

  • 🟩 execution_time [-19.735µs; -15.005µs] or [-5.992%; -4.555%]

scenario:SpanBench/benchDatadogAPI

  • 🟩 execution_time [-3.474µs; -2.549µs] or [-16.923%; -12.415%]

scenario:SpanBench/benchDatadogAPI-opcache

  • 🟩 execution_time [-2.343µs; -0.908µs] or [-11.643%; -4.511%]

scenario:SpanBench/benchOpenTelemetryAPI

  • 🟩 execution_time [-13.408µs; -8.765µs] or [-4.068%; -2.659%]

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead-opcache

  • 🟥 execution_time [+5.988µs; +10.457µs] or [+2.385%; +4.166%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-15.171µs; -10.629µs] or [-7.653%; -5.361%]

scenario:TraceSerializationBench/benchSerializeTrace-opcache

  • 🟩 execution_time [-16.279µs; -14.221µs] or [-8.957%; -7.824%]

PROFeNoM
PROFeNoM previously approved these changes Feb 27, 2024
Copy link
Contributor

@PROFeNoM PROFeNoM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This micro-optimization has some really nice improvements 😃

@PROFeNoM PROFeNoM self-requested a review February 27, 2024 07:20
@PROFeNoM PROFeNoM dismissed their stale review February 27, 2024 07:25

Hum, there seems to be an issue compiling ddtelemetry (Rust).

@bwoebi bwoebi force-pushed the bob/micro-optimize-shall_log branch 2 times, most recently from bd4e1ec to c533eb6 Compare February 27, 2024 11:30
@bwoebi bwoebi force-pushed the bob/micro-optimize-shall_log branch from c533eb6 to b436003 Compare February 27, 2024 15:25
@bwoebi bwoebi merged commit 552b0ef into master Feb 28, 2024
597 checks passed
@bwoebi bwoebi deleted the bob/micro-optimize-shall_log branch February 28, 2024 11:21
@github-actions github-actions bot added this to the 0.99.0 milestone Feb 28, 2024
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.

None yet

3 participants