Skip to content

perf(profiling): avoid copy of func name when utf8#3700

Merged
morrisonlevi merged 1 commit intomasterfrom
levi/refactor-function-name
Mar 12, 2026
Merged

perf(profiling): avoid copy of func name when utf8#3700
morrisonlevi merged 1 commit intomasterfrom
levi/refactor-function-name

Conversation

@morrisonlevi
Copy link
Copy Markdown
Collaborator

@morrisonlevi morrisonlevi commented Mar 11, 2026 •
edited
Loading

Description

While doing some refactoring, I noticed that we can avoid one copy of the function name:

    // All or nearly all functions should be valid UTF8, so we're going to
    // pessimize the error case to avoid having to make copies on the happy
    // case.
    let string = if core::str::from_utf8(&buffer).is_ok() {
        // SAFETY: we just validate it's valid UTF-8.
        // We can re-use the buffer rather than copying it into a new string.
        unsafe { String::from_utf8_unchecked(buffer) }
    } else {
        // This will repeat some work but it's okay, and this will make a copy.
        String::from_utf8_lossy(&buffer).into_owned()
    };

We can optimize the error path if we care to, but not right now.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions Bot added profiling Relates to the Continuous Profiler tracing labels Mar 11, 2026
@datadog-prod-us1-5
Copy link
Copy Markdown

datadog-prod-us1-5 Bot commented Mar 11, 2026 •
edited by datadog-datadog-prod-us1 Bot
Loading

⚠️ Tests

✨ Fix all issues with BitsAI or with Cursor

⚠️ Warnings

🧪 1028 Tests failed

testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Datadog) (Fix with Cursor) 
DDTrace\Tests\Integration\PHPInstallerTest::testSearchPhpBinaries
Test code or tested code printed unexpected output: Searching for available php binaries, this operation might take a while.
testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Datadog) (Fix with Cursor) 
Risky Test
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:97
testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor) 
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 69b1813f00000000afb7d58f7f0cc81a
tid: 69b1813f00000000
hexProcessTraceId: afb7d58f7f0cc81a
hexProcessSpanId: b12f4db56fa60379
processTraceId: 12661823689664022554
processSpanId: 12767508910276215673
View all

ℹ️ Info

❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 185b372 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Mar 11, 2026 •
edited
Loading

Benchmarks [ profiler ]

Benchmark execution time: 2026-03-11 14:50:06

Comparing candidate commit 185b372 in PR branch levi/refactor-function-name with baseline commit 7d767af in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 30 metrics, 6 unstable metrics.

Base automatically changed from levi/cargo-fmt to master March 11, 2026 07:03
@morrisonlevi
perf(profiling): avoid copy of func name when utf8
185b372 
This does pessimize it when it isn't utf8, but nearly all functions
will be utf8.
@morrisonlevi morrisonlevi force-pushed the levi/refactor-function-name branch from ded3ce4 to 185b372 Compare March 11, 2026 14:32
@morrisonlevi morrisonlevi added Overhead Relates to latency, CPU, or memory overhead and removed tracing labels Mar 11, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 11, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.30%. Comparing base (7d767af) to head (185b372).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3700      +/-   ##
==========================================
- Coverage   62.40%   62.30%   -0.11%     
==========================================
  Files         142      142              
  Lines       13586    13586              
  Branches     1775     1775              
==========================================
- Hits         8479     8465      -14     
- Misses       4301     4314      +13     
- Partials      806      807       +1

see 3 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 7d767af...185b372. 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.

@morrisonlevi morrisonlevi marked this pull request as ready for review March 11, 2026 16:00
@morrisonlevi morrisonlevi requested a review from a team as a code owner March 11, 2026 16:00
@morrisonlevi morrisonlevi changed the title perf(profiling): avoid copy of func name perf(profiling): avoid copy of func name when utf8 Mar 11, 2026
realFlowControl
realFlowControl approved these changes Mar 12, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@realFlowControl realFlowControl left a comment

Choose a reason for hiding this comment

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

Nice 🚀

@morrisonlevi morrisonlevi merged commit 237080d into master Mar 12, 2026
2059 of 2063 checks passed
@morrisonlevi morrisonlevi deleted the levi/refactor-function-name branch March 12, 2026 14:54
@github-actions github-actions Bot added this to the 1.17.0 milestone Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@realFlowControl realFlowControl realFlowControl approved these changes

Assignees

No one assigned

Labels

Overhead Relates to latency, CPU, or memory overhead profiling Relates to the Continuous Profiler

Projects

None yet

Milestone

1.17.0

Development

Successfully merging this pull request may close these issues.

3 participants

@morrisonlevi @codecov-commenter @realFlowControl