Zend: Preallocate error buffer with capacity tracking by devnexen · Pull Request #20565 · php/php-src

devnexen · 2025-11-23T09:09:16Z

Replace separate num_errors/errors fields with a single struct containing size, capacity, and a flexible array. The buffer grows by 50% when needed instead of reallocating on every recorded error.

Add ZEND_ERR_BUF_SIZE macro and zend_init_errors_buf() helper
Allocate error buffer in zend_startup for NTS builds
Remove redundant NULL checks since buffer is always allocated

ndossche · 2025-11-23T10:22:34Z

It's often a good idea to choose a factor of 1.5, there's a mathematical reason behind it (https://github.com/facebook/folly/blob/main/folly/docs/FBVector.md#memory-handling).

devnexen · 2025-11-23T10:40:36Z

yes will try ; even though usage and growth might not be that as critical as your (nice) link contexts.

Girgias · 2025-11-23T12:43:52Z

It's often a good idea to choose a factor of 1.5, there's a mathematical reason behind it (https://github.com/facebook/folly/blob/main/folly/docs/FBVector.md#memory-handling).

Interesting, we currently extend arrays by a factor of 2, I wonder what would happen if we do 1.5 instead.

ndossche · 2025-11-23T13:30:02Z

Interesting, we currently extend arrays by a factor of 2, I wonder what would happen if we do 1.5 instead.

Won't work because the hash masking depends on it being a power of 2.

TimWolla · 2025-11-23T22:53:10Z

Can we get a clearer PR title?

ndossche · 2026-01-04T12:21:19Z

Zend/zend.c

-		 * Use pow2 realloc if it becomes a problem. */
 		EG(num_errors)++;
-		EG(errors) = erealloc(EG(errors), sizeof(zend_error_info*) * EG(num_errors));
+		// pondering if uint32_t is more appropriate but would need to align the allocation size then


Storing the size like this is very ugly in my opinion. What you could do is make a struct with a size field and a flexible array member at the end.

ndossche

When dealing with nested fields in module globals, we normally use the EG(errors)->size etc syntax instead of EG(errors->size). Please change this.

I'm not sure how I feel about unconditionally allocating a buffer in the executor globals. I understand this is to allow unconditional access to fields like EG(errors)->size ? I see previously it was always safe to access EG(num_errors).

ndossche · 2026-01-24T21:45:35Z

Zend/zend_globals.h

 	HashTable callable_convert_cache;

 	void *reserved[ZEND_MAX_RESERVED_RESOURCES];
+	struct zend_err_buf *errors;


I don't believe this should be at the end, please place this under record_errors

no it does not need to.

ndossche · 2026-01-24T21:46:14Z

Zend/zend.c

-		EG(errors)[EG(num_errors)-1] = info;
+		EG(errors->size)++;
+		if (EG(errors->size) >= EG(errors->capacity)) {
+			// not sure we can get high number of errors so safe `might be` over cautious here


Please just drop this comment

ndossche · 2026-01-24T21:48:30Z

Zend/zend.c


+static void zend_init_errors_buf(zend_executor_globals *eg)
+{
+	eg->errors = pemalloc(ZEND_ERR_BUF_SIZE(2), true);


Why is this persistent?

well it needs to live long enough. I m not yet that familiar with Zend engine (why I m doing this PR) though. I ll try in non persistent mode just in case..

devnexen · 2026-01-24T21:59:04Z

I'm not sure how I feel about unconditionally allocating a buffer in the executor globals. I understand this is to allow unconditional access to fields like EG(errors)->size ? I see previously it was always safe to access EG(num_errors).

Yes this is the reason, remember we re dealing with a "flexible" array now.

devnexen · 2026-03-19T20:47:55Z

ping :)

arnaud-lb

This is a good idea.

I've added some comments. I also noticed that Nora's comments weren't addressed yet (factor of 1.5, EG(errors)->size instead of EG(errors->size)).

arnaud-lb · 2026-03-30T10:19:39Z

Zend/zend_globals.h

 	bool record_errors;
-	uint32_t num_errors;
-	zend_error_info **errors;
+	struct zend_err_buf *errors;


Suggestion:

Suggested change

struct zend_err_buf *errors;

struct zend_err_buf errors;

And change zend_error_info *buf[1] to zend_error_info **errors in struct zend_err_buf.

This would simplify the realloc logic (we only need to reallocate EG(errors).errors), as well as initialization (EG(errors) can be zeroed).

arnaud-lb · 2026-03-30T10:20:11Z

Zend/zend.c

+#define COMPILED_STRING_DESCRIPTION_FORMAT "%s(%u) : %s"

 ZEND_API char *zend_make_compiled_string_description(const char *name) /* {{{ */
 {
 	const char *cur_filename;
-	int cur_lineno;
+	uint32_t cur_lineno;


Please avoid unrelated changes

Replace separate num_errors/errors fields with a single struct containing size, capacity, and a flexible array. The buffer grows by 50% when needed instead of reallocating on every recorded error. - Add ZEND_ERR_BUF_SIZE macro and zend_init_errors_buf() helper - Allocate error buffer in zend_startup for NTS builds - Remove redundant NULL checks since buffer is always allocated

devnexen · 2026-03-30T11:27:04Z

This is a good idea.

I've added some comments. I also noticed that Nora's comments weren't addressed yet (factor of 1.5, EG(errors)->size instead of EG(errors->size)).

note factor of 1.5 was already addressed for a while

EG(errors).capacity + (EG(errors).capacity >> 1

- Change EG(errors) from pointer to embedded struct (arnaud-lb) - Replace flexible array member with plain pointer (arnaud-lb) - Use EG(errors).field syntax instead of EG(errors->field) (ndossche) - Remove persistent allocation, use erealloc for the inner array - Remove zend_init_errors_buf/ZEND_ERR_BUF_SIZE, zeroing suffices - Free EG(errors).errors buffer in zend_free_recorded_errors - Save/restore full EG(errors) state in error handler paths - Revert unrelated int->uint32_t change (arnaud-lb)

arnaud-lb · 2026-03-30T13:10:30Z

Zend/zend.c

 }
 /* }}} */

+


Unexpected white space change

arnaud-lb · 2026-03-30T13:11:29Z

Zend/zend.c

+	uint32_t orig_num_errors = 0;
+	uint32_t orig_cap_errors = 0;
+	zend_error_info **orig_errors = NULL;


Here it maybe possible to replace these 3 variables by a single zend_err_buf variable

arnaud-lb · 2026-03-30T13:12:18Z

Zend/zend.c

+		uint32_t num_errors = EG(errors).size;
+		uint32_t cap_errors = EG(errors).capacity;
+		zend_error_info **errors = EG(errors).errors;


arnaud-lb · 2026-03-30T13:15:37Z

Zend/zend.c

-	if (!EG(num_errors)) {
-		return;
-	}


We could keep this check to avoid the memset() and other checks when there are not errors

- Save/restore EG(errors) with struct copy instead of 3 separate vars - Remove extra blank line left from zend_init_errors_buf removal - Restore early return in zend_free_recorded_errors to skip memset

github-actions bot added the Category: Engine label Nov 23, 2025

devnexen marked this pull request as ready for review November 23, 2025 13:55

devnexen requested a review from dstogov as a code owner November 23, 2025 13:55

devnexen changed the title ~~Zend little upd~~ Zend internal changes: error handling and zend_make_compiled_string_description() takes in account lineno is uint32_t Nov 24, 2025

devnexen requested a review from ndossche December 5, 2025 23:35

ndossche reviewed Jan 4, 2026

View reviewed changes

devnexen marked this pull request as draft January 4, 2026 20:09

devnexen force-pushed the zend_little_upd branch from d9f84e7 to c6bed1f Compare January 4, 2026 20:09

github-actions bot added Extension: opcache ABI break labels Jan 4, 2026

devnexen force-pushed the zend_little_upd branch 8 times, most recently from f38b0e3 to 078130f Compare January 5, 2026 07:15

devnexen force-pushed the zend_little_upd branch 7 times, most recently from 21c2fdc to 5111ca5 Compare January 17, 2026 15:11

devnexen marked this pull request as ready for review January 17, 2026 15:48

devnexen requested a review from ndossche January 17, 2026 15:49

ndossche requested changes Jan 24, 2026

View reviewed changes

devnexen force-pushed the zend_little_upd branch 3 times, most recently from ecc36fc to 3ac9616 Compare January 24, 2026 23:12

devnexen requested a review from arnaud-lb March 30, 2026 09:15

arnaud-lb reviewed Mar 30, 2026

View reviewed changes

devnexen added 2 commits March 30, 2026 11:58

review

155e86f

devnexen force-pushed the zend_little_upd branch from 3ac9616 to 890f548 Compare March 30, 2026 11:08

devnexen marked this pull request as draft March 30, 2026 11:28

devnexen force-pushed the zend_little_upd branch from 890f548 to 480d331 Compare March 30, 2026 11:29

devnexen force-pushed the zend_little_upd branch from 480d331 to 49c8d34 Compare March 30, 2026 11:54

devnexen marked this pull request as ready for review March 30, 2026 12:43

arnaud-lb reviewed Mar 30, 2026

View reviewed changes

Address review feedback: use struct copy for save/restore, cleanup.

4bb157d

- Save/restore EG(errors) with struct copy instead of 3 separate vars - Remove extra blank line left from zend_init_errors_buf removal - Restore early return in zend_free_recorded_errors to skip memset

Conversation

devnexen commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ndossche commented Nov 23, 2025

Uh oh!

devnexen commented Nov 23, 2025

Uh oh!

Girgias commented Nov 23, 2025

Uh oh!

ndossche commented Nov 23, 2025

Uh oh!

TimWolla commented Nov 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ndossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

devnexen commented Mar 19, 2026

Uh oh!

arnaud-lb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devnexen commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

devnexen commented Nov 23, 2025 •

edited

Loading

devnexen commented Jan 24, 2026 •

edited

Loading