WIP POC for better perf. #734
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| ]; | ||
|
|
||
| if (!isset($levels[$level])) { | ||
| fwrite(STDERR, "Invalid level: {$level}\n"); |
There was a problem hiding this comment.
Perhaps you can use our lib commands used for CLI as
Console::error()
There was a problem hiding this comment.
I think we can go a few steps further, using a work queue like we now do for reads. Then regardless of whether it's string IDs or Documents, or how many levels, we can write every relationship level as an array instead of falling back to one-by-one. Will make the logic a lot simpler if we only have one path as well
| /** | ||
| * Check if bulk relationship write optimizations are enabled. | ||
| * Controlled via environment variable DB_RELATIONSHIP_BULK_WRITES (default: enabled). | ||
| * | ||
| * @return bool | ||
| */ | ||
| private function shouldUseRelationshipBulkWrites(): bool | ||
| { | ||
| $val = getenv('DB_RELATIONSHIP_BULK_WRITES'); | ||
| if ($val === false || $val === '') { | ||
| return true; | ||
| } | ||
| $val = strtolower((string)$val); | ||
| return !in_array($val, ['0', 'false', 'off'], true); | ||
| } | ||
|
|
There was a problem hiding this comment.
Let's remove, if we're going to add a better way, it can just be default
| if ($attrValue instanceof Document || | ||
| (is_array($attrValue) && !empty($attrValue) && isset($attrValue[0]) && $attrValue[0] instanceof Document)) { |
There was a problem hiding this comment.
This won't handle the case of nested string IDs
| $sql = " | ||
| UPDATE {$this->getSQLTable($collectionId)} | ||
| SET {$setSQL} | ||
| WHERE {$this->quote('_uid')} IN ({$inSQL}){$tenantClause} | ||
| "; |
There was a problem hiding this comment.
What's the scenario where we want to set the same values on many documents but not use updateDocuments?
| if ($hasNestedRelationships) { | ||
| // Use original method for nested relationships - handles everything including links |
There was a problem hiding this comment.
Doesn't this defeat most of the benefit? Will we only get improvements for a single level relationship where all are strings IDs?
| // Ensure rich docs (without nesting) one-by-one | ||
| foreach ($richDocsNoNesting as $relatedDoc) { |
There was a problem hiding this comment.
Benchmarks should go in the bin dir as CLI tasks
3 participants
Optimize relationship writes with bulk operations.
Adds bulk insert/update methods to avoid N+1 queries when creating relationships. Junction table links now use batch inserts, and one-to-many/many-to-one relationships use
SQL UPDATEwithINclauses instead of individual updates. IncludesupdateManyByIds()adapter method and benchmarks.Controlled via
DB_RELATIONSHIP_BULK_WRITESenv var (enabled by default) for before/after tests in benchmarks.