Skip to content

docs(roadmap): PHP preload/warmup plan (P3) — pre-implementation#4

Draft
andypost wants to merge 6 commits intoroadmapfrom
claude/php-preload-warmup-EA2PX
Draft

docs(roadmap): PHP preload/warmup plan (P3) — pre-implementation#4
andypost wants to merge 6 commits intoroadmapfrom
claude/php-preload-warmup-EA2PX

Conversation

@andypost
Copy link
Copy Markdown
Owner

Summary

Pre-implementation plan for roadmap item P3 — PHP preload/warmup hook (roadmap/unit-php.md:72-77). No code yet; this PR exists to review the design before cutting patches.

Adds roadmap/plan-php-preload-warmup.md covering:

  • Preload: new "preload" config key mapped to opcache.preload via sapi_module_struct.ini_entries, injected between sapi_startup() and php_module_startup() in nxt_php_setup() so PHP actually reads it.
  • Warmup: new "warmup": [...] array eagerly passed to opcache_compile_file() after targets resolve in nxt_php_start(), before the worker accepts requests. Soft failures only.
  • Error policy: preload = PHP-native fatal (worker restart); warmup = log + continue.
  • Extensive test matrix: validation negatives, happy paths, missing file, syntax error, opcache-disabled, targets interaction, reload, unicode/symlinks, 200-script stress, options.file conflict.
  • Shared scaffolding with /run + scheduler (roadmap/plan-run.md): array-of-string validator, generation-bump reload path, control-API dispatcher pattern. On-demand /control/applications/<name>/warmup endpoint and port RPC deferred to P7 to keep P3 landable in ~3 days.

Critical files identified

  • src/nxt_php_sapi.c (lines 313-487, 690-780, 1014-1045)
  • src/nxt_application.h:62-65
  • src/nxt_main_process.c:236-248
  • src/nxt_conf_validation.c:1049, 3238, 3499-3518
  • New: test/test_php_preload.py, test/test_php_warmup.py, test/php/preload/, test/php/warmup/

Test plan

  • Review design choices, especially ini_entries injection timing and warmup relative-path semantics (resolves against first target's root).
  • Confirm soft-failure policy for warmup (log + skip vs. fatal).
  • Confirm deferring on-demand /warmup endpoint to P7 is acceptable.
  • After approval: implement per §File-by-File and run the test matrix described in the plan.

Draft PR — will close or convert to ready once implementation lands in follow-up commits.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request proposes an implementation plan for PHP preload and warmup hooks to eliminate cold-start spikes by pre-compiling scripts. The review feedback suggests optimizing the warmup loop's error handling to ensure robustness, warns about potential preloading failures when running as root, and recommends that Unit configuration settings override php.ini directives to maintain consistency.

Comment thread roadmap/plan-php-preload-warmup.md Outdated
Comment on lines +42 to +43
`opcache.preload_user` defaults to `conf->user`; overridable via `options.admin`
if the user wants a different uid.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Note that PHP's opcache.preload will fail if the process is running as root and opcache.preload_user is not set to a non-root user. If conf->user defaults to root (e.g., if Unit is started as root and no user is configured for the app), preloading will fail. It might be worth adding a validation check or a warning if the resulting user is root.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — folded in d8b96a3. Plan now: opcache.preload_user defaults to conf->user, falls back to the worker's effective uid when conf->user is empty, and if the resolved user would be root (uid 0) we emit a loud WARN and skip the injection so the worker stays up rather than crashing at php_module_startup. Verified locally against PHP 8.4.19: bad preload paths produce PHP Fatal error: Failed opening required '…' at startup, which matches what our negative-case alert regex will key on.


Generated by Claude Code

Comment on lines +60 to +62
Warmup failures are soft: missing file → WARN + skip; syntax error → catch via
`zend_try`/`zend_catch`, clear, WARN + skip; `opcache_compile_file` symbol
absent (opcache disabled) → one WARN, skip whole list.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To ensure that a syntax error in one file doesn't prevent the rest of the list from being warmed up, the zend_try / zend_catch block should be placed inside the loop iterating over the warmup array. If placed outside the loop, any bailout triggered by opcache_compile_file will terminate the entire warmup process for that worker.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Agreed, real bug in the plan — fixed in d8b96a3. The zend_try { … } zend_catch { … } zend_end_try(); now wraps each loop iteration individually, not the outer loop. One bailout from opcache_compile_file skips only the failing entry and sibling entries continue to compile. Both the "High-Level Approach" prose and the src/nxt_php_sapi.c insertion-site description have been updated to make the per-iteration scoping explicit.


Generated by Claude Code

Comment thread roadmap/plan-php-preload-warmup.md Outdated
|---|---|
| `preload` path missing/unreadable | PHP module-startup FAILURE → worker exit → Unit restart (existing path). Alert references `opcache.preload`. |
| `preload` relative, no workdir | Validation error. |
| `options.file` also sets `opcache.preload` | `options.file` wins for every directive; our `ini_entries` only fills in what the file left blank. Log INFO on conflict. |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There is a contradiction between the conflict policy here and the risk mitigation on line 236. This table states that options.file should win, but line 236 suggests logging an error if the active value doesn't match the Unit configuration. In standard PHP SAPI behavior, sapi_module_struct.ini_entries are processed after php.ini and thus act as overrides. It is recommended to let the Unit configuration key win, as this is more consistent with Unit's management of other application options and avoids the complexity of pre-parsing the INI file to detect conflicts.

Suggested change
| `options.file` also sets `opcache.preload` | `options.file` wins for every directive; our `ini_entries` only fills in what the file left blank. Log INFO on conflict. |
| `options.file` also sets `opcache.preload` | Unit configuration key wins (standard SAPI behavior); `ini_entries` overrides `options.file`. Log INFO on conflict. |

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Accepted — flipped the policy in d8b96a3. You're right on both counts: PHP's SAPI parses ini_entries after php.ini, so our injection naturally overrides options.file; and this lines up with Unit's existing convention where config keys (user, group, working_directory) beat file-supplied values. Removes the need to pre-parse the user's ini to detect conflicts. Error Policy table row now reads "Unit preload key wins … INFO log on detected conflict." The "Reviewer Burden & Binary Compatibility" §Opt-in guarantees section was aligned to the same story.


Generated by Claude Code

andypost pushed a commit that referenced this pull request Apr 20, 2026
Three Gemini Code Assist review threads folded into the plan:

c1 (root-guard): preload user defaults to conf->user, then worker uid;
    if resolved user is root (uid 0), WARN and skip injection rather
    than letting PHP hard-fail at startup.

c2 (zend_try placement): per-iteration try/catch around each
    opcache_compile_file() call, not the outer loop — a bailout from
    one entry no longer terminates the rest of the warmup list.

c3 (options.file precedence): flip to Unit-key-wins; matches PHP SAPI
    semantics (ini_entries parsed after php.ini) and Unit convention
    (config-key beats file for user/group/working_directory). Removes
    need to pre-parse the ini file.

Also strip the trailing blank line that failed check-whitespace on
the previous commit.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
andypost pushed a commit that referenced this pull request Apr 20, 2026
Add two new keys to the PHP application config:

- "preload": absolute path to the opcache.preload script, injected into
  sapi_module_struct.ini_entries before php_module_startup so the engine
  honors it. Resolves relative paths against the working directory and
  derives opcache.preload_user from conf->user (skips + WARNs if the
  resolved user would be root, which PHP refuses).

- "warmup": array of scripts eagerly compiled with opcache_compile_file()
  after targets resolve and before the worker accepts requests.
  Compile-only, never executed. Wrapped in per-iteration
  zend_try/zend_catch so one bad entry does not abort the rest of the
  list. Missing files, syntax errors, and a missing symbol all soft-fail
  with a WARN.

Unit's preload key takes precedence over opcache.preload set via
options.file because SAPI ini_entries are applied after php.ini.

Addresses PR #4 review comments c1 (root guard), c2 (per-iteration
bailout scope) and c3 (precedence).

Refs roadmap/plan-php-preload-warmup.md.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
andypost pushed a commit that referenced this pull request Apr 22, 2026
Add two new keys to the PHP application config:

- "preload": absolute path to the opcache.preload script, injected into
  sapi_module_struct.ini_entries before php_module_startup so the engine
  honors it. Resolves relative paths against the working directory and
  derives opcache.preload_user from conf->user (skips + WARNs if the
  resolved user would be root, which PHP refuses).

- "warmup": array of scripts eagerly compiled with opcache_compile_file()
  after targets resolve and before the worker accepts requests.
  Compile-only, never executed. Wrapped in per-iteration
  zend_try/zend_catch so one bad entry does not abort the rest of the
  list. The whole loop runs inside php_request_startup/shutdown so the
  executor globals are live when opcache compiles each file (without
  this the helper silently returns FALSE and nothing is actually
  primed). Missing files, syntax errors, and a missing symbol all
  soft-fail with a WARN.

Unit's preload key takes precedence over opcache.preload set via
options.file because SAPI ini_entries are applied after php.ini.

Tests in test/test_php_preload.py cover validation, preload happy path
+ root-user guard + options.file override, warmup caching (asserts
X-Warmup: a.php=1;b.php=1;c.php=1), warmup soft-fail on missing +
syntax-error + opcache-disabled, reload-bumps-generation, and array-
index addressability (PUT, DELETE warmup/N). Fixtures under
test/php/preload/. 21 new tests + 7 regression tests (test_php_application
-k 'preload or opcache') all pass against PHP 8.4 embed SAPI.

Addresses three Gemini Code Assist review threads on PR #4:
  c1 root-guard, c2 per-iteration zend_try, c3 Unit-key-wins precedence.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
@andypost andypost force-pushed the claude/php-preload-warmup-EA2PX branch from 0ec7281 to a22f083 Compare April 22, 2026 14:04
andypost and others added 2 commits April 22, 2026 14:23
Add planning documents covering the fork's direction and priorities:

Roadmap docs:
- README.md — index and navigation hub
- unit-roadmap.md — cross-cutting platform work, core daemon, governance
- unit-maintainer.md — maintainer-facing synthesis, priorities, backlog
- unit-php.md — PHP ZTS worker pool, persistent worker, TrueAsync
- unit-python.md — free-threaded 3.13t, subinterpreters, ASGI/WSGI
- unit-ruby.md — thread pool, Ractors, Fiber scheduler, YJIT
- unit-cron.md — scheduler/cron primitive for framework tasks
- unit-arm32.md — armv7/armhf SIGBUS/alignment investigation
- unit-todos.md — ~90 TODO/FIXME/HACK markers inventory
- unit-wasm.md — WASM backends, WASI component model, OCI distribution

Core changes:
- nxt_conf.h — add new config validation helpers
- nxt_conf_validation.c — expand validation for routes, targets, TLS
- nxt_controller.c — wire up new validation entry points

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
claude added 2 commits April 22, 2026 14:24
Pre-implementation plan for roadmap item P3 covering config schema,
SAPI injection points, error policy, test matrix, and shared
scaffolding with the /run + scheduler plan.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
Add two new keys to the PHP application config:

- "preload": absolute path to the opcache.preload script, injected into
  sapi_module_struct.ini_entries before php_module_startup so the engine
  honors it. Resolves relative paths against the working directory and
  derives opcache.preload_user from conf->user (skips + WARNs if the
  resolved user would be root, which PHP refuses).

- "warmup": array of scripts eagerly compiled with opcache_compile_file()
  after targets resolve and before the worker accepts requests.
  Compile-only, never executed. Wrapped in per-iteration
  zend_try/zend_catch so one bad entry does not abort the rest of the
  list. The whole loop runs inside php_request_startup/shutdown so the
  executor globals are live when opcache compiles each file (without
  this the helper silently returns FALSE and nothing is actually
  primed). Missing files, syntax errors, and a missing symbol all
  soft-fail with a WARN.

Unit's preload key takes precedence over opcache.preload set via
options.file because SAPI ini_entries are applied after php.ini.

Tests in test/test_php_preload.py cover validation, preload happy path
+ root-user guard + options.file override, warmup caching (asserts
X-Warmup: a.php=1;b.php=1;c.php=1), warmup soft-fail on missing +
syntax-error + opcache-disabled, reload-bumps-generation, and array-
index addressability (PUT, DELETE warmup/N). Fixtures under
test/php/preload/. 21 new tests + 7 regression tests (test_php_application
-k 'preload or opcache') all pass against PHP 8.4 embed SAPI.

Addresses three Gemini Code Assist review threads on PR #4:
  c1 root-guard, c2 per-iteration zend_try, c3 Unit-key-wins precedence.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
@andypost andypost force-pushed the claude/php-preload-warmup-EA2PX branch from a22f083 to be2315c Compare April 22, 2026 14:25
claude added 2 commits April 23, 2026 11:24
The source build via make -C pkg/contrib .wasmtime downloads the
155 MB wasmtime-v43.0.1-src.tar.gz and drives a cmake/Rust build
that typically takes 30-45 min on ubuntu-latest runners.  Under
flaky mirror conditions (packages.freeunit.org currently 403,
occasional GitHub CDN blips) the whole PR goes red with a useless
"Process completed with exit code 2" failure before any real
verification happens.

Switch to the prebuilt x86_64-linux-c-api tarball shipped on the
same GitHub release (~13 MB, ~30 s).  Matches the pattern already
in .github/workflows/ci-dev-distro-compiler.yaml for the alpine
and fedora jobs.  The pkg/contrib source build is still exercised
by workflow_dispatch runs of ci-dev-distro-compiler.yaml, so the
packaging path keeps its own coverage — just not on every PR.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
Released 2026-04-20 (a few days after 43.0.1).  Notable upstream
changes:

  - Performance improvement on aarch64 by removing the csdb instruction
    by default
  - Multiple WASI-TLS crates consolidated into a single crate with
    feature flags
  - cranelift-codegen now compiles for no_std
  - demangle feature now compatible with no_std targets
  - Build minimum bumped to Rust 1.92.0

c-api ABI surface unchanged: Unit's auto/modules/wasm probe
(wasm_config_new + wasm_config_delete) compiles + links fine against
the v44 prebuilt c-api tarball.  Verified locally:

  ./configure wasm \
    --include-path=.../wasmtime-v44.0.0-x86_64-linux-c-api/include \
    --lib-path=.../wasmtime-v44.0.0-x86_64-linux-c-api/lib
  make wasm

builds wasm.unit.so without warnings.

Files updated:
  - pkg/contrib/src/wasmtime/version: WASMTIME_VERSION := 44.0.0
  - pkg/contrib/src/wasmtime/SHA512SUMS: new digest for src.tar.gz
  - .github/workflows/ci.yml: 3 references to v43.0.1 -> v44.0.0

Note: src/wasm-wasi-component/Cargo.toml still pins wasmtime 35.0.0;
that crate-vs-c-api skew is tracked separately in roadmap/unit-wasm.md
and isn't bumped here.

https://claude.ai/code/session_01FDhY6aDxweH1CJE8Ma5BfK
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.

2 participants