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, so that's why we have to grab it again on first rinit. 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.

We also make another optimization as part of this: we borrow memory from the config when possible for the decoder. Examples include otel normalization when the result is a static string like "0" or "1", as well as when using system environment variables.

Parts of zai config had to be refactored. The internals were very brittle to changes. This PR refines the tests and hopefully improves documentation as well so that it's a bit less so.

Overall, we shaved off roughly 40-50% of the time spent in zai_config_rinit. Here's an example diff from the profiling comparison view:

Note that you can see sapi_getenv in the flamegraph now where you couldn't before. However, we still come out ahead.

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

🧪 13 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)

DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 69bc468a00000000681003afd8dad8d0
tid: 69bc468a00000000
hexProcessTraceId: 681003afd8dad8d0
hexProcessSpanId: 2679377e84e46c49
processTraceId: 7498497433364256976
processSpanId: 2772308062158220361

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: 69bc4783000000002392baba6eb144cd
tid: 69bc478300000000
hexProcessTraceId: 2392baba6eb144cd
hexProcessSpanId: b61db3f5a8e8e1ed
processTraceId: 2563316447811028173
processSpanId: 13122842756909687277

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: 5970886 | 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 68.73%. Comparing base (9cde556) to head (5970886).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3670      +/-   ##
==========================================
- Coverage   68.80%   68.73%   -0.08%     
==========================================
  Files         166      166              
  Lines       19030    19030              
  Branches     1797     1797              
==========================================
- Hits        13094    13080      -14     
- Misses       5122     5135      +13     
- Partials      814      815       +1

Flag	Coverage Δ
helper-rust-integration	`78.82% <ø> (ø)`
helper-rust-unit	`49.36% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.
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 9cde556...5970886. 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-19 20:02:07

Comparing candidate commit 5970886 in PR branch levi/cache-getenv with baseline commit 9cde556 in branch master.

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

scenario:BM_TeaSapiSpindown

🟩 execution_time [-25.000µs; -11.625µs] or [-4.626%; -2.151%]

scenario:EmptyFileBench/benchEmptyFileDdprof

🟩 execution_time [-417.800µs; -170.680µs] or [-10.341%; -4.225%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

🟥 execution_time [+2.458µs; +3.502µs] or [+2.355%; +3.356%]

scenario:SamplingRuleMatchingBench/benchRegexMatching2

🟥 execution_time [+40.004ns; +99.396ns] or [+2.758%; +6.852%]

scenario:SymfonyBench/benchSymfonyBaseline

🟩 execution_time [-296.988µs; -184.052µs] or [-4.212%; -2.610%]

scenario:SymfonyBench/benchSymfonyDdprof-opcache

🟩 execution_time [-321.616µs; -190.864µs] or [-3.591%; -2.131%]

scenario:SymfonyBench/benchSymfonyOverhead

🟩 execution_time [-306.675µs; -180.085µs] or [-3.666%; -2.153%]

zend_abstract_interface/config/config.c

bwoebi · 2026-03-13T17:54:11Z

Good job on refactoring the zai_env stuff, that's much more readable now.

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.

- Remove !env_to_ini_name guard from zai_config_ini_rinit so SAPI env vars (e.g. Apache SetEnv) are always consulted at RINIT, not only when env_to_ini_name is NULL. This restores ini->modified being set from env vars, which the Rust RC client relies on to detect user-set values before applying dynamic config overrides. - Add SAPI env check to zai_config_find_and_set_value at first RINIT so values are visible before zai_config_ini_rinit runs. Code such as the signal handler setup for DD_TRACE_HEALTH_METRICS_ENABLED (Apache SetEnv) reads decoded config between first_time_rinit and zai_config_ini_rinit and requires SAPI env to be present at that point. - Remove erroneous manual zai_config_first_time_rinit(true) call in the perdir/multi-consumer INI test; REQUEST_BEGIN already triggers it through the extension RINIT handler. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

zend_abstract_interface/config/config_ini.c

bwoebi

Looks good to me, nice job Levi :-)

…dd-update * 'master' of github.com:DataDog/dd-trace-php: feat(sidecar): add thread mode as fallback connection for restricted environments (#3573) Migrate deprecated GitLab runner tags (#3715) Adds process tags to remote config payload (#3658) perf(config): cache sys getenv (#3670) Fixes the tag name for process tags (#3709) Fix debugger ephemerals handling (#3685) Fix #3651: Prevent crash during shutdown in Frankenphp (#3662) Add dynamic instrumentation and exception replay to startup logging (#3667) chore: bump bytes crate from 1.9.0 to 1.11.1 to address CVE-2026-25541 (#3669) Merge pull request #3701 from DataDog/brian.marks/add-ksr-tag ci: fix Windows job flakiness caused by dirty workspace (#3694) Fixup CI owner association (#3704) Add Rust rewrite of the AppSec helper alongside the C++ implementation Remove debug instruction Fix script order debug Fix exploration logic chore(ci): add final_status property on junit XML [APMSP-2610] Fix DD_TRACE_SYMFONY_HTTP_ROUTE=false Optimize Symfony http.route caching with path map approach

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