Skip to content

Conversation

@nielsdos
Copy link
Member

@nielsdos nielsdos commented Feb 9, 2025

Review commits one by one. This reduces memory usage, uses cheaper string functions, and cleans up the code a bit.

This simplifies the code and reduces memory usage.
These checks are always false because we're receiving a valid
zend_object.
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

MSTM Another packing suggestion

Comment on lines 65 to 68
bool initialised;
sqlite3 *db;
php_sqlite3_func *funcs;
php_sqlite3_collation *collations;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't you move this next to the exception field to back it? As there are going to be 7 bytes of padding, and then again a bunch of padding after the exception field.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't do it originally because it seemed better to keep db and initialised in the same cacheline, but I see now that I can just move exception up and it will result in the same struct size while keeping db and initialised together.

@nielsdos nielsdos merged commit 2acda55 into php:master Feb 10, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants