Skip to content

Get rid of some bitwise checking in ddog_shall_log#2539

Merged
bwoebi merged 2 commits intomasterfrom
bob/micro-optimize-shall_log
Feb 28, 2024
Merged

Get rid of some bitwise checking in ddog_shall_log#2539
bwoebi merged 2 commits intomasterfrom
bob/micro-optimize-shall_log

Conversation

@bwoebi
Copy link
Copy Markdown
Collaborator

@bwoebi bwoebi commented Feb 26, 2024

Description

Micro-optimization for logging

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@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
Copy Markdown

codecov-commenter commented Feb 26, 2024

Codecov Report

❌ Patch coverage is 37.41497% with 92 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.54%. Comparing base (380774d) to head (b436003).
⚠️ Report is 865 commits behind head on master.

Files with missing lines Patch % Lines
ext/ddtrace.c 6.25% 45 Missing ⚠️
ext/hook/uhook_legacy.c 50.00% 8 Missing ⚠️
ext/signals.c 0.00% 8 Missing ⚠️
ext/dogstatsd_client.c 0.00% 6 Missing ⚠️
ext/hook/uhook.c 72.22% 5 Missing ⚠️
ext/auto_flush.c 33.33% 4 Missing ⚠️
ext/comms_php.c 33.33% 2 Missing ⚠️
ext/configuration.c 0.00% 2 Missing ⚠️
ext/engine_hooks.c 0.00% 2 Missing ⚠️
ext/excluded_modules.c 0.00% 2 Missing ⚠️
... and 6 more

❌ Your patch status has failed because the patch coverage (37.41%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2539      +/-   ##
============================================
- Coverage     78.30%   76.54%   -1.76%     
  Complexity      267      267              
============================================
  Files           112      138      +26     
  Lines         13458    17459    +4001     
  Branches          0      976     +976     
============================================
+ Hits          10538    13364    +2826     
- Misses         2920     3575     +655     
- Partials          0      520     +520     
Flag Coverage Δ
appsec-extension 69.09% <ø> (?)
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 with missing lines 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 34 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...b436003. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link
Copy Markdown

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
Copy Markdown
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
@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.

3 participants