Skip to content

perf: reduce database document and conversion allocations #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loks0n merged 4 commits into from
Sep 8, 2026
Merged

perf: reduce database document and conversion allocations #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e05c9ba
perf: avoid copying unchanged Document arrays
loks0n Sep 7, 2026
540ee27
refactor: simplify Document export without reflection
loks0n Sep 8, 2026
b59acc9
perf: reduce database validation and conversion allocations
loks0n Sep 8, 2026
753f116
test: remove AllocationTest
loks0n Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Database/Adapter/Mongo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,7 @@ public function castingAfter(Document $collection, Document $document): Document
$value = [$value];
}

foreach ($value as &$node) {
foreach ($value as $index => $node) {
switch ($type) {
case Database::VAR_INTEGER:
case Database::VAR_BIGINT:
Expand All @@ -1384,8 +1384,8 @@ public function castingAfter(Document $collection, Document $document): Document
default:
break;
}
$value[$index] = $node;
}
unset($node);
$document->setAttribute($key, ($array) ? $value : $value[0]);
}

Expand Down Expand Up @@ -1468,7 +1468,7 @@ public function castingBefore(Document $collection, Document $document): Documen
$value = [$value];
}

foreach ($value as &$node) {
foreach ($value as $index => $node) {
switch ($type) {
case Database::VAR_DATETIME:
if (!($node instanceof UTCDateTime)) {
Expand All @@ -1485,8 +1485,8 @@ public function castingBefore(Document $collection, Document $document): Documen
default:
break;
}
$value[$index] = $node;
}
unset($node);
$document->setAttribute($key, ($array) ? $value : $value[0]);
}
$indexes = $collection->getAttribute('indexes');
Expand Down
4 changes: 2 additions & 2 deletions src/Database/Adapter/Postgres.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2038,8 +2038,8 @@ protected function decodeArray(array $value): string
return '{}';
}

foreach ($value as &$item) {
$item = '"' . str_replace(['"', '(', ')'], ['\"', '\(', '\)'], $item) . '"';
foreach ($value as $index => $item) {
$value[$index] = '"' . str_replace(['"', '(', ')'], ['\"', '\(', '\)'], $item) . '"';
}

return '{' . implode(",", $value) . '}';
Expand Down
75 changes: 42 additions & 33 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9315,7 +9315,11 @@ protected function removeUnknownAttributes(Document $collection, Document $docum
}

$dropped = [];
foreach (\array_keys($document->getArrayCopy()) as $key) {
$documentKeys = [];
foreach ($document as $key => $value) {
$documentKeys[] = $key;
}
foreach ($documentKeys as $key) {
if (\str_starts_with($key, '$') || isset($known[$key])) {
continue;
}
Expand Down Expand Up @@ -9396,12 +9400,14 @@ public function encode(Document $collection, Document $document, bool $applyDefa
$value = ($array) ? $value : [$value];
}

foreach ($value as $index => $node) {
if ($node !== null) {
foreach ($filters as $filter) {
$node = $this->encodeAttribute($filter, $node, $document);
if (!empty($filters)) {
foreach ($value as $index => $node) {
if ($node !== null) {
foreach ($filters as $filter) {
$node = $this->encodeAttribute($filter, $node, $document);
}
$value[$index] = $node;
}
$value[$index] = $node;
}
}

Expand Down Expand Up @@ -9499,9 +9505,10 @@ public function decode(Document $collection, Document $document, array $selectio
|| \in_array($key, $selections)
|| \in_array('*', $selections);

if ($selected || $hasRelationshipSelections) {
if (!empty($filters) && ($selected || $hasRelationshipSelections)) {
$filters = \array_reverse($filters);
foreach ($value as $index => $node) {
foreach (\array_reverse($filters) as $filter) {
foreach ($filters as $filter) {
$node = $this->decodeAttribute($filter, $node, $document, $key);
}
$value[$index] = $node;
Expand Down Expand Up @@ -9573,33 +9580,35 @@ public function casting(Document $collection, Document $document): Document
$value = [$value];
}

foreach ($value as $index => $node) {
switch ($type) {
case self::VAR_ID:
// Disabled until Appwrite migrates to use real int ID's for MySQL
//$type = $this->adapter->getIdAttributeType();
//\settype($node, $type);
$node = (string)$node;
break;
case self::VAR_BOOLEAN:
$node = (bool)$node;
break;
case self::VAR_INTEGER:
$node = (int)$node;
break;
case self::VAR_BIGINT:
if (\is_string($node) && BigIntValidator::fitsPhpInt($node, $signed)) {
if (\in_array($type, [self::VAR_ID, self::VAR_BOOLEAN, self::VAR_INTEGER, self::VAR_BIGINT, self::VAR_FLOAT], true)) {
foreach ($value as $index => $node) {
switch ($type) {
case self::VAR_ID:
// Disabled until Appwrite migrates to use real int ID's for MySQL
//$type = $this->adapter->getIdAttributeType();
//\settype($node, $type);
$node = (string)$node;
break;
case self::VAR_BOOLEAN:
$node = (bool)$node;
break;
case self::VAR_INTEGER:
$node = (int)$node;
}
break;
case self::VAR_FLOAT:
$node = (float)$node;
break;
default:
break;
}
break;
case self::VAR_BIGINT:
if (\is_string($node) && BigIntValidator::fitsPhpInt($node, $signed)) {
$node = (int)$node;
}
break;
case self::VAR_FLOAT:
$node = (float)$node;
break;
default:
break;
}

$value[$index] = $node;
$value[$index] = $node;
}
}

$document->setAttribute($key, ($array) ? $value : $value[0]);
Expand Down
23 changes: 11 additions & 12 deletions src/Database/Document.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,21 @@ public function __construct(array $input = [])
continue;
}

$converted = false;
foreach ($value as $childKey => $child) {
// An array value is either a list of nested sub-documents or a list of
// plain items (dates, numbers, strings): wrap the former, leave the latter.
// is_array() tells them apart and avoids array-accessing a non-array
// value (e.g. a UTCDateTime), which would otherwise fatal.
if (\is_array($child) && (isset($child['$id']) || isset($child['$collection']))) {
$value[$childKey] = new self($child);
$converted = true;
}
}

$input[$key] = $value;
if ($converted) {
$input[$key] = $value;
}
}

parent::__construct($input);
Expand Down Expand Up @@ -430,7 +434,7 @@ public function getArrayCopy(array $allow = [], array $disallow = []): array

$output = [];

foreach ($array as $key => &$value) {
foreach ($array as $key => $value) {
if (!empty($allow) && !\in_array($key, $allow)) { // Export only allow fields
continue;
}
Expand All @@ -442,17 +446,12 @@ public function getArrayCopy(array $allow = [], array $disallow = []): array
if ($value instanceof self) {
$output[$key] = $value->getArrayCopy($allow, $disallow);
} elseif (\is_array($value)) {
foreach ($value as $childKey => &$child) {
if ($child instanceof self) {
$output[$key][$childKey] = $child->getArrayCopy($allow, $disallow);
} else {
$output[$key][$childKey] = $child;
}
}
$value = \array_map(
fn ($item) => $item instanceof self ? $item->getArrayCopy($allow, $disallow) : $item,
$value
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Comment on lines +449 to 453

@greptile-apps greptile-apps Bot Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Scalar Arrays Are Rebuilt

The unconditional array_map() rebuilds every exported array, even when it contains only scalar values. The same pattern in __clone() also copies scalar-only arrays instead of retaining their copy-on-write storage. For large fields, this reintroduces allocation overhead that the PR is intended to avoid. The revised test raises the allocation allowance and removes clone allocation coverage rather than preserving the earlier optimization.

Knowledge Base Used: Document lifecycle and representation

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/Database/Document.php
Line: 449-453

Comment:
**Scalar Arrays Are Rebuilt**

The unconditional `array_map()` rebuilds every exported array, even when it contains only scalar values. The same pattern in `__clone()` also copies scalar-only arrays instead of retaining their copy-on-write storage. For large fields, this reintroduces allocation overhead that the PR is intended to avoid. The revised test raises the allocation allowance and removes clone allocation coverage rather than preserving the earlier optimization.

**Knowledge Base Used:** [Document lifecycle and representation](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/utopia-php/database/-/docs/document-lifecycle.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

@loks0n loks0n Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a deliberate tradeoff: we measured the conditional ReflectionReference approach and chose the simpler implementation without reflection. Scalar arrays still get copied on export, but removing reference iteration avoids allocating a PHP reference wrapper for every element. In the 2,000-operation scalar-array export benchmark, requested allocation bytes fell from 112,928,000 on the base implementation to 41,040,000 with this version. The reflection-based version allocated less, but we are accepting that difference for simplicity.

Cloning retains the base branch's implementation; this PR no longer claims a clone allocation improvement. The revised export test checks the optimization we are shipping (avoiding reference-allocation overhead), rather than requiring copy-on-write sharing that we deliberately removed. It fails on the base implementation and passes here. Behavior tests retain explicit-reference detachment and nested-document isolation coverage.

The PR description now explains this scope and the measurements: the complete PR reduces allocation events by 25.83% and requested Zend bytes by 17.28% on the local API workload. We will keep the simpler version and are not restoring ReflectionReference for this PR.

if (empty($value)) {
$output[$key] = $value;
}
$output[$key] = $value;
} else {
$output[$key] = $value;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Database/Validator/Query/Order.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class Order extends Base
{
/**
* @var array<int|string, mixed>
* @var array<int|string, true>
*/
protected array $schema = [];

Expand All @@ -19,7 +19,7 @@ class Order extends Base
public function __construct(array $attributes = [], protected bool $supportForAttributes = true)
{
foreach ($attributes as $attribute) {
$this->schema[$attribute->getAttribute('key', $attribute->getAttribute('$id'))] = $attribute->getArrayCopy();
$this->schema[$attribute->getAttribute('key', $attribute->getAttribute('$id'))] = true;
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Database/Validator/Query/Select.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class Select extends Base
{
/**
* @var array<int|string, mixed>
* @var array<int|string, true>
*/
protected array $schema = [];

Expand All @@ -34,7 +34,7 @@ class Select extends Base
public function __construct(array $attributes = [], protected bool $supportForAttributes = true)
{
foreach ($attributes as $attribute) {
$this->schema[$attribute->getAttribute('key', $attribute->getAttribute('$id'))] = $attribute->getArrayCopy();
$this->schema[$attribute->getAttribute('key', $attribute->getAttribute('$id'))] = true;
}
}

Expand Down
90 changes: 90 additions & 0 deletions tests/unit/DocumentTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,4 +417,94 @@ public function testEmptyDocumentSequence(): void
$this->assertNull($empty->getSequence());
$this->assertNotSame('', $empty->getSequence());
}
public function testConstructionPreservesScalarArraysAndConvertsOnlyDocuments(): void
{
$object = new \stdClass();
$input = [
'empty' => [],
'values' => [7 => 'text', 'null' => null, 'bool' => false, 'object' => $object],
'child' => ['$id' => 'child', 'name' => 'nested'],
'children' => ['first' => ['$id' => 'first'], 9 => 'plain'],
];
$document = new Document($input);

$this->assertSame([], $document->getAttribute('empty'));
$this->assertSame($input['values'], $document->getAttribute('values'));
$this->assertSame('child', $document->getAttribute('child')->getId());
$this->assertSame('first', $document->getAttribute('children')['first']->getId());
$this->assertSame('plain', $document->getAttribute('children')[9]);
$this->assertSame(['$id' => 'first'], $input['children']['first']);
}

public function testArrayCopyPreservesKeysAndFiltersNestedDocuments(): void
{
$document = new Document([
'name' => 'parent',
'secret' => 'hidden',
'values' => [7 => 'seven', 'null' => null, 'empty' => []],
'children' => ['child' => new Document(['name' => 'nested', 'secret' => 'hidden'])],
]);
$copy = $document->getArrayCopy(['name', 'secret', 'values', 'children'], ['secret']);

$this->assertSame([
'name' => 'parent',
'values' => [7 => 'seven', 'null' => null, 'empty' => []],
'children' => ['child' => ['name' => 'nested']],
], $copy);
$copy['values'][7] = 'changed';
$copy['children']['child']['name'] = 'changed';
$this->assertSame('seven', $document->getAttribute('values')[7]);
$this->assertSame('nested', $document->getAttribute('children')['child']->getAttribute('name'));
}

public function testClonePreservesScalarKeysAndIsolatesNestedDocuments(): void
{
$object = new \stdClass();
$original = new Document([
'empty' => [],
'values' => [7 => 'seven', 'object' => $object],
'children' => ['child' => new Document(['name' => 'nested']), 9 => 'plain'],
]);
$copy = clone $original;
$copy['values'][7] = 'changed';
$copy->getAttribute('children')['child']->setAttribute('name', 'changed');

$this->assertSame([], $copy->getAttribute('empty'));
$this->assertSame([7, 'object'], array_keys($copy->getAttribute('values')));
$this->assertSame($object, $copy->getAttribute('values')['object']);
$this->assertSame('seven', $original->getAttribute('values')[7]);
$this->assertSame('nested', $original->getAttribute('children')['child']->getAttribute('name'));
$this->assertSame('plain', $copy->getAttribute('children')[9]);
}

public function testArrayCopyAndCloneDetachReferencedArrayElements(): void
{
$scalar = 'before';
$nested = ['value' => 'before'];
$document = new Document(['values' => ['first' => &$scalar, 7 => &$nested, 'last' => false]]);
$export = $document->getArrayCopy();
$clone = clone $document;
$scalar = 'after';
$nested['value'] = 'after';

$expected = ['first' => 'before', 7 => ['value' => 'before'], 'last' => false];
$this->assertSame($expected, $export['values']);
$this->assertSame($expected, $clone->getAttribute('values'));
$this->assertSame('after', $document->getAttribute('values')['first']);
}

public function testScalarArrayExportAvoidsReferenceAllocationOverhead(): void
{
$document = new Document(['values' => range(1, 100_000)]);
memory_reset_peak_usage();
$before = memory_get_usage();
$copy = $document->getArrayCopy();
$allocated = memory_get_peak_usage() - $before;

$this->assertCount(100_000, $copy['values']);
$this->assertLessThan(3 * 1024 * 1024, $allocated, 'Export should copy the array without wrapping every element in a reference');
$copy['values'][0] = 0;
$this->assertSame(1, $document->getAttribute('values')[0]);
}

}
Loading