From 2a2f2ae07c9652b3444c6931e5b85e0913042edf Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 4 Aug 2026 10:23:56 +0200 Subject: [PATCH] perf(magicmapper): check the once-per-process table guard before doing work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every insert, update and find path in MagicMapper calls ensureTableForRegisterSchema(), so it runs once per OBJECT OPERATION rather than once per register+schema. Importing OpenCatalogi's configuration produced 1,137 calls for 225 object saves. Each call built a table name, resolved two slugs and wrote an info log — and then reached the identical fast path inside handleExistingTable() and returned true. The guard already existed; it was just evaluated after the work it was meant to avoid. Hoisted verbatim: same two conditions, same meaning, checked first. Both conditions are load-bearing. hasRegisterSchemaChanged() compares the stored schema VERSION, so a schema updated mid-process — an import bumping a version and then saving objects against it — still falls through and re-syncs columns. isTableColumnsVerified() is process-scoped, so every fresh request verifies at least once. `force` bypasses both, unchanged. This does not reduce how often callers CALL the method. It makes the redundant calls cost two comparisons instead of a log write and an existence check. Measured on the same repair, same instance: 1,137 -> 405 log lines, a 64% drop. The remaining 405 are legitimate — first call per register+schema, plus schemas whose version genuinely changed during the import. 1361 Db tests, 4312 assertions. --- lib/Db/MagicMapper/MagicTableHandler.php | 30 +++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/Db/MagicMapper/MagicTableHandler.php b/lib/Db/MagicMapper/MagicTableHandler.php index 165c57f55b..789555b562 100644 --- a/lib/Db/MagicMapper/MagicTableHandler.php +++ b/lib/Db/MagicMapper/MagicTableHandler.php @@ -93,11 +93,39 @@ public function __construct( */ public function ensureTableForRegisterSchema(Register $register, Schema $schema, bool $force=false): bool { - $tableName = $this->getTableNameForRegisterSchema(register: $register, schema: $schema); $registerId = $register->getId(); $schemaId = $schema->getId(); $cacheKey = $this->magicMapper->getCacheKey(registerId: $registerId, schemaId: $schemaId); + // Verified once per process, checked FIRST. + // + // Every insert, update and find path in MagicMapper calls this method, so + // it runs once per object operation rather than once per register+schema. + // Importing OpenCatalogi's configuration produced 1,137 calls for 225 + // object saves — and each one built a table name, resolved two slugs, and + // wrote an info log before reaching the identical fast path inside + // handleExistingTable() and returning true. + // + // This is that exact predicate, hoisted: same two conditions, same + // meaning, evaluated before the work rather than after it. It does not + // reduce how often callers CALL this method; it makes the redundant calls + // cost two comparisons instead of a log write and an existence check. + // + // Both conditions are load-bearing and neither may be dropped. + // hasRegisterSchemaChanged() compares the stored schema VERSION, so a + // schema updated mid-process (an import bumping a version, then saving + // objects against it) still falls through and re-syncs its columns. + // isTableColumnsVerified() is process-scoped, so a fresh request always + // re-verifies at least once. `force` bypasses both, as before. + if ($force === false + && MagicMapper::isTableColumnsVerified(cacheKey: $cacheKey) === true + && $this->magicMapper->hasRegisterSchemaChanged(register: $register, schema: $schema) === false + ) { + return true; + } + + $tableName = $this->getTableNameForRegisterSchema(register: $register, schema: $schema); + $this->logger->info( message: '[MagicTableHandler] Creating/updating table for register+schema', context: [