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

🧪 1632 Tests failed

testWebserverReload from custom-framework-autoloading-test.ApacheReloadTest (Datadog) (Fix with Cursor)

ApacheReloadTest::testWebserverReload
Got error from webserver:
[Fri Mar 13 15:47:59 2026]  Script:  'tests/Frameworks/Custom/Version_Not_Autoloaded/index.php'
/usr/local/src/php/ext/filter/filter.c(357) :  Freeing 0x00007bca49a7d000 (16 bytes), script=tests/Frameworks/Custom/Version_Not_Autoloaded/index.php
Last leak repeated 15 times
=== Total 16 memory leaks detected ===
172.18.72.25:9998 127.0.0.1 - - [13/Mar/2026:15:47:59 +0000] "GET / HTTP/1.1" 200 181 "-" "-"
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.18.72.25. Set the 'ServerName' directive globally to suppress this message
[Fri Mar 13 15:48:00 2026]  Script:  'tests/Frameworks/Custom/Version_Not_Autoloaded/index.php'
/usr/local/src/php/ext/filter/filter.c(357) :  Freeing 0x00007bca49a7d000 (16 bytes), script=tests/Frameworks/Custom/Version_Not_Autoloaded/index.php
...

testTracingDisabledByPerdirConfig from custom-framework-autoloading-test.DDTrace\Tests\Integrations\Custom\NotAutoloaded\EarlyDisableTest (Datadog) (Fix with Cursor)

DDTrace\Tests\Integrations\Custom\NotAutoloaded\EarlyDisableTest::testTracingDisabledByPerdirConfig
Got error from webserver:
[Fri Mar 13 15:48:09 2026]  Script:  'tests/Frameworks/Custom/Version_Not_Autoloaded/per-dir-disabled/test.php'
/usr/local/src/php/ext/filter/filter.c(357) :  Freeing 0x00007bca49a7d000 (16 bytes), script=tests/Frameworks/Custom/Version_Not_Autoloaded/per-dir-disabled/test.php
Last leak repeated 15 times
=== Total 16 memory leaks detected ===
172.18.72.25:9998 127.0.0.1 - - [13/Mar/2026:15:48:09 +0000] "GET /per-dir-disabled/test.php HTTP/1.1" 200 192 "-" "-"

tests/Common/WebFrameworkTestCase.php:74
tests/Common/MultiPHPUnitVersionAdapter_typed.php:33
...

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

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

View all

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

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

🔗 Commit SHA: a12cb93 | 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.40%. Comparing base (7d767af) to head (41fc47c).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3670      +/-   ##
==========================================
+ Coverage   62.39%   62.40%   +0.01%     
==========================================
  Files         142      142              
  Lines       13586    13586              
  Branches     1775     1775              
==========================================
+ Hits         8477     8479       +2     
+ Misses       4304     4301       -3     
- Partials      805      806       +1

see 1 file 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...41fc47c. 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-03-13 16:43:19

Comparing candidate commit a12cb93 in PR branch levi/cache-getenv with baseline commit 7d767af in branch master.

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

scenario:EmptyFileBench/benchEmptyFileDdprof-opcache

🟩 execution_time [-337.594µs; -106.526µs] or [-8.781%; -2.771%]

scenario:SymfonyBench/benchSymfonyBaseline-opcache

🟩 execution_time [-265.726µs; -169.474µs] or [-3.727%; -2.377%]

scenario:SymfonyBench/benchSymfonyOverhead

🟩 execution_time [-246.026µs; -166.414µs] or [-2.964%; -2.005%]

scenario:TraceSerializationBench/benchSerializeTrace

🟩 execution_time [-26.132µs; -13.968µs] or [-6.011%; -3.213%]

scenario:TraceSerializationBench/benchSerializeTrace-opcache

🟩 execution_time [-14.039µs; -7.361µs] or [-4.037%; -2.116%]

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

A non cli sapi check would be fine by me too. Just let's not eat the cost for cli.

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.

bwoebi · 2026-03-02T12:48:45Z

zend_abstract_interface/config/config.c

-    // We want to explicitly allow pre-RINIT access to env vars here. So that callers can have an early view at config.
-    // But in general allmost all configurations shall only be accessed after first RINIT. (the trivial getter will
-    return zai_getenv_ex(name, buf, true) == ZAI_ENV_SUCCESS;
+    if (zai_getenv(name, buf) == ZAI_ENV_SUCCESS) {


Suggested change

if (zai_getenv(name, buf) == ZAI_ENV_SUCCESS) {

if (zai_getenv_ex(name, buf, false, false) == ZAI_ENV_SUCCESS) {

? Because you defined zai_getenv() as always accessing the process env.

zai_config_find_and_set_value is called only by zai_config_first_time_rinit. At this time, we do want to get the process environment, yes?

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.

Replace zai_getenv_ex/zai_getenv with two focused functions: - zai_sapi_getenv: borrows value from the SAPI environment - zai_sys_getenv: borrows value from the process environment Both return zai_option_str instead of writing into a caller-supplied buffer, eliminating the ZAI_ENV_BUFFER_* error codes and the scratch buffer indirection throughout callers. Update config, config_ini, and otel_config to use the new API. Rename zai_config_get_cached_env_value to zai_config_sys_getenv_cached to return zai_option_str directly. Add ZAI_OPTION_STRL macro and zai_option_str_eq helper to string.h. Remove now-obsolete error.cc env tests.

bwoebi · 2026-03-13T17:48:38Z

zend_abstract_interface/config/config.c

+            // +1 to hold the null terminator.
+            char *dst = pemalloc(len + 1, 1);
+            // +1 to include the null terminator in the copy.
+            memcpy(dst, val.ptr, len + 1);


Suggested change

// +1 to hold the null terminator.

char *dst = pemalloc(len + 1, 1);

// +1 to include the null terminator in the copy.

memcpy(dst, val.ptr, len + 1);

char *dst = zend_strndup(val.ptr, len);

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 2 times, most recently from 58b7933 to 091c262 Compare February 27, 2026 14:42

bwoebi reviewed Mar 2, 2026

View reviewed changes

morrisonlevi force-pushed the levi/cache-getenv branch from 29395b8 to 5890581 Compare March 10, 2026 17:23

morrisonlevi added 10 commits March 11, 2026 14:14

perf(config): avoid interim string copies

95ce7cc

fix: zero name len test

f2bfaa3

fix: missed a getenv -> getenv_ex conversion

851505d

perf: skip getenv cache refresh on CLI

c475ec3

test: sapi precedence and cached sys env

183a5c5

test: better output on zval str failure

3bfaaac

test: fix headers/build for zai (hopefully)

a4c8f30

test: fix sapi_getenv mock

583f0af

test: try to fix sapi env priority

41fc47c

morrisonlevi force-pushed the levi/cache-getenv branch from 54de6fc to 41fc47c Compare March 11, 2026 20:14

realFlowControl self-requested a review March 13, 2026 16:02

bwoebi reviewed Mar 13, 2026

View reviewed changes

	if (zai_getenv(name, buf) == ZAI_ENV_SUCCESS) {
	if (zai_getenv_ex(name, buf, false, false) == ZAI_ENV_SUCCESS) {

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 by datadog-prod-us1-6 bot 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:EmptyFileBench/benchEmptyFileDdprof-opcache

scenario:SymfonyBench/benchSymfonyBaseline-opcache

scenario:SymfonyBench/benchSymfonyOverhead

scenario:TraceSerializationBench/benchSerializeTrace

scenario:TraceSerializationBench/benchSerializeTrace-opcache

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 Mar 2, 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!

bwoebi Mar 2, 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 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

bwoebi Mar 13, 2026

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 by datadog-prod-us1-6 bot

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

bwoebi Mar 2, 2026 •

edited

Loading