[Appla]

This pull request adds a new INI directive, v8js.thread_pool_size (default 0), enabling users to configure how many background worker threads the V8 platform should use.

The directive accepts values from 0 to 64 (V8 currently enforces a hard limit of 16). Invalid values or attempts to modify the setting after V8 initialization are rejected.

In process‑based environments such as FPM pools, a single worker thread is often sufficient, whereas V8’s default behavior uses NCPU - 1. This option allows deployments to tune thread usage explicitly based on workload and resource constraints.

[Copilot]

🟡 Changes recommended

Strict parsing and regression-test coverage remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds the v8js.thread_pool_size INI directive to configure V8 background worker threads.

Changes:

• Adds directive parsing and validation.
• Stores the setting in process globals.
• Applies it during V8 platform initialization.

File summaries

File	Summary	Findings
v8js_v8.cc	Applies the configured thread pool size during V8 initialization.	None reported.
v8js_main.cc	Registers and validates the new INI directive.	Moderate (3 votes): PHP 8.0/8.1 parsing accepts invalid numeric prefixes. Nit (3 votes): add PHPT coverage for valid, invalid, and post-initialization changes.
php_v8js_macros.h	Adds process-global storage for the setting.	None reported.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Two moderate review items remain unresolved in the test coverage and package manifest.

Review details

Suppressed comments (2)

tests/thread_pool_size.phpt:36

• This test only verifies the INI string and then initializes V8 with the final value 4; it never observes the platform's worker count. A regression that ignores the value and always calls the no-argument NewDefaultPlatform() would still pass, so please add an assertion or test hook that verifies the configured count reaches the platform construction path.

// 4. Initialize V8 platform by creating an instance
$v8 = new V8Js();
var_dump($v8->executeString('1 + 2'));

tests/thread_pool_size.phpt:2

• The PEAR manifest explicitly enumerates the test files (package.xml:38-215), but it has no entry for tests/thread_pool_size.phpt. A PECL package built from that manifest will therefore omit this new regression test; add the manifest entry and checksum alongside this file.

--TEST--
v8js.thread_pool_size INI directive validation and lifecycle

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite