perf(config): cache sys getenv by morrisonlevi · Pull Request #3670 · DataDog/dd-trace-php

morrisonlevi · 2026-02-24T09:48:34Z

Background

In request initialization aka rinit/activate, calling the system getenv when a SAPI env var isn't found is slow enough it shows up in profiles:

Note that glibc's getenv does a linear scan on the environ and does strncmp on the members, so we can definitely do better.

Description

Caches libc's getenv in minit and again in first rinit. Note that php-fpm's env directive actually changes the system env var, it doesn't include it in the fastcgi environment. This change is also more correct: system environment variables map to a system INI setting, which shouldn't change at runtime, so if they do, we shouldn't really respect it.

Here's an example diff from the profiling comparison view:

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

datadog-official · 2026-02-24T09:50:23Z

⚠️ Tests

✨ Fix all issues with BitsAI or with Cursor

⚠️ Warnings

🧪 1028 Tests failed

testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Datadog) (Fix with Cursor)

Risky Test
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:97

testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Datadog) (Fix with Cursor)

DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 69a22005000000005a63336598720ed5
tid: 69a2200500000000
hexProcessTraceId: 5a63336598720ed5
hexProcessSpanId: cb8d35ddefbfbb9d
processTraceId: 6513105997550194389
processSpanId: 14667438788741938077

phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106

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: 69a2207c00000000c66bf1941a8a2df6
tid: 69a2207c00000000
hexProcessTraceId: c66bf1941a8a2df6
hexProcessSpanId: 2803203ded0a756a
processTraceId: 14297787060420488694
processSpanId: 2883183636789228906

phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106

View all

ℹ️ Info

❄️ No new flaky tests detected

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 4630694 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

codecov-commenter · 2026-02-24T09:53:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.13%. Comparing base (6733f8f) to head (4630694).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3670      +/-   ##
==========================================
- Coverage   62.20%   62.13%   -0.07%     
==========================================
  Files         141      141              
  Lines       13352    13352              
  Branches     1746     1746              
==========================================
- Hits         8305     8296       -9     
- Misses       4256     4263       +7     
- Partials      791      793       +2

see 4 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 6733f8f...4630694. 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 · 2026-02-24T11:02:52Z

Benchmarks [ tracer ]

Benchmark execution time: 2026-02-28 00:01:12

Comparing candidate commit 4630694 in PR branch levi/cache-getenv with baseline commit 6733f8f in branch master.

Found 2 performance improvements and 1 performance regressions! Performance is the same for 188 metrics, 3 unstable metrics.

scenario:MessagePackSerializationBench/benchMessagePackSerialization

🟩 execution_time [-7.327µs; -5.673µs] or [-6.651%; -5.149%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

🟩 execution_time [-9.642µs; -7.798µs] or [-8.594%; -6.950%]

scenario:SamplingRuleMatchingBench/benchRegexMatching1

🟥 execution_time [+26.202ns; +98.798ns] or [+2.051%; +7.732%]

bwoebi · 2026-02-27T10:02:16Z

zend_abstract_interface/config/config.c

    (void)in_request;
 #endif

+    if (in_request) {


We only need it for FPM, right? So let's add a sapi_module check here?
Saving some minor time on CLI scripts.

I'm not sure what litespeed does, and IIRC we did have a customer using it. How about a not-CLI SAPI check instead? That way web SAPIs stay consistent, but it's skipped on CLI where it's unnecessary.

I think it's reasonably clearer now. Patch is smaller, at least xD

bwoebi · 2026-02-27T10:17:21Z

zend_abstract_interface/config/config.c

+    if (!cached->ptr) return false;
+    if (cached->len >= buf.len) return false;
+
+    memcpy(buf.ptr, cached->ptr, cached->len + 1);


There's not really an intrinsic need to copy. The copying in zai_getenv_ex is also unnecessary, but here we very clearly don't need it (but yeah, you'll have to duplicate branches then).
I'd personally return a simple zai_str instead of zai_env_buffer.

bwoebi · 2026-02-27T10:18:24Z

zend_abstract_interface/config/config.c

+typedef struct {
+    // ptr == NULL means env var was unset at MINIT.
+    // ptr != NULL with len == 0 means env var was set to an empty string.
+    char *ptr;
+    size_t len;
+} zai_config_cached_env_value;
+
+static zai_config_cached_env_value zai_config_cached_env_values[ZAI_CONFIG_ENTRIES_COUNT_MAX][ZAI_CONFIG_NAMES_COUNT_MAX];


Suggested change

typedef struct {

// ptr == NULL means env var was unset at MINIT.

// ptr != NULL with len == 0 means env var was set to an empty string.

char *ptr;

size_t len;

} zai_config_cached_env_value;

static zai_config_cached_env_value zai_config_cached_env_values[ZAI_CONFIG_ENTRIES_COUNT_MAX][ZAI_CONFIG_NAMES_COUNT_MAX];

static zai_string zai_config_cached_env_values[ZAI_CONFIG_ENTRIES_COUNT_MAX][ZAI_CONFIG_NAMES_COUNT_MAX];

Simply?

zai_string isn't supposed to have a null pointer, right? IIRC that's why I did this. This distinguishes between unset and null. But there is a zai_option_str, and I think using a zai_option_str except actually owning it is better than putting null into zai_string. So I'll experiment with that.

In rinit, calling system getenv when a SAPI env var isn't found is slow enough it shows up in profiles. glibc's getenv does a linear scan on the environ and does strncmp on the members, so we can definitely do better. We save it in minit so it's available for SYSTEM_INI variables and again on the first request. This is also arguably more correct: system environment variables map to a system INI setting, which shouldn't change at runtime.

morrisonlevi added the cat:performance label Feb 24, 2026

morrisonlevi changed the title ~~perf(config): caches sys getenv on minit~~ perf(config): cache sys getenv on minit Feb 24, 2026

morrisonlevi changed the title ~~perf(config): cache sys getenv on minit~~ perf(config): cache sys getenv Feb 24, 2026

morrisonlevi force-pushed the levi/cache-getenv branch 2 times, most recently from 5a292ea to e26db1d Compare February 25, 2026 12:04

morrisonlevi marked this pull request as ready for review February 27, 2026 09:47

morrisonlevi requested a review from a team as a code owner February 27, 2026 09:47

bwoebi reviewed Feb 27, 2026

View reviewed changes

morrisonlevi force-pushed the levi/cache-getenv branch from e26db1d to 58b7933 Compare February 27, 2026 14:38

morrisonlevi force-pushed the levi/cache-getenv branch from 58b7933 to 091c262 Compare February 27, 2026 14:42

perf(config): avoid interim string copies

4630694

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(config): cache sys getenv#3670

perf(config): cache sys getenv#3670
morrisonlevi wants to merge 2 commits intomasterfrom
levi/cache-getenv

morrisonlevi commented Feb 24, 2026 •

edited

Loading

Uh oh!

datadog-official bot commented Feb 24, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Feb 24, 2026 •

edited

Loading

Uh oh!

pr-commenter bot commented Feb 24, 2026 •

edited

Loading

Uh oh!

bwoebi Feb 27, 2026

Uh oh!

morrisonlevi Feb 27, 2026 •

edited

Loading

Uh oh!

morrisonlevi Feb 27, 2026

Uh oh!

bwoebi Feb 27, 2026 •

edited

Loading

Uh oh!

bwoebi Feb 27, 2026

Uh oh!

morrisonlevi Feb 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

morrisonlevi commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Description

Reviewer checklist

Uh oh!

datadog-official bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

ℹ️ Info

Uh oh!

codecov-commenter commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ tracer ]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

scenario:SamplingRuleMatchingBench/benchRegexMatching1

Uh oh!

bwoebi Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

morrisonlevi Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morrisonlevi Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

bwoebi Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwoebi Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

morrisonlevi Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

morrisonlevi commented Feb 24, 2026 •

edited

Loading

datadog-official bot commented Feb 24, 2026 •

edited

Loading

codecov-commenter commented Feb 24, 2026 •

edited

Loading

pr-commenter bot commented Feb 24, 2026 •

edited

Loading

morrisonlevi Feb 27, 2026 •

edited

Loading

bwoebi Feb 27, 2026 •

edited

Loading

morrisonlevi Feb 27, 2026 •

edited

Loading