From f7601878a4c915189d915df7a8d0fd36b6ec6a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 17 Jun 2024 09:33:03 +0200 Subject: [PATCH] perf: Use generator/count query on cleanup tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Command/ResetDocument.php | 7 ++++--- lib/Cron/Cleanup.php | 6 ++---- lib/Db/DocumentMapper.php | 20 ++++++++++++++++++-- lib/Db/SessionMapper.php | 11 +++++++++++ lib/Migration/ResetSessionsBeforeYjs.php | 14 +++----------- lib/Service/DocumentService.php | 6 +++++- lib/Service/SessionService.php | 4 ++++ 7 files changed, 47 insertions(+), 21 deletions(-) diff --git a/lib/Command/ResetDocument.php b/lib/Command/ResetDocument.php index 4702db69a..e9588409c 100644 --- a/lib/Command/ResetDocument.php +++ b/lib/Command/ResetDocument.php @@ -78,9 +78,10 @@ class ResetDocument extends Command { } if ($all) { - $fileIds = array_map(static function (Document $document) { - return $document->getId(); - }, $this->documentService->getAll()); + $fileIds = []; + foreach ($this->documentService->getAll() as $document) { + $fileIds[] = $document->getId(); + } } else { $fileIds = [$fileId]; } diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index e374e742f..d049ef816 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -56,10 +56,8 @@ class Cleanup extends TimedJob { */ protected function run($argument): void { $this->logger->debug('Run cleanup job for text documents'); - $documents = $this->documentService->getAll(); - foreach ($documents as $document) { - $allSessions = $this->sessionService->getAllSessions($document->getId()); - if (count($allSessions) > 0) { + foreach ($this->documentService->getAll() as $document) { + if ($this->sessionService->countAllSessions($document->getId()) > 0) { // Do not reset if there are any sessions left // Inactive sessions will get removed further down and will trigger a reset next time continue; diff --git a/lib/Db/DocumentMapper.php b/lib/Db/DocumentMapper.php index 13b878b7d..81099deb9 100644 --- a/lib/Db/DocumentMapper.php +++ b/lib/Db/DocumentMapper.php @@ -23,6 +23,7 @@ namespace OCA\Text\Db; +use Generator; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -55,12 +56,27 @@ class DocumentMapper extends QBMapper { return Document::fromRow($data); } - public function findAll(): array { + public function findAll(): Generator { $qb = $this->db->getQueryBuilder(); $result = $qb->select('*') ->from($this->getTableName()) ->executeQuery(); + try { + while ($row = $result->fetch()) { + yield $this->mapRowToEntity($row); + } + } finally { + $result->closeCursor(); + } + } - return $this->findEntities($qb); + public function countAll(): int { + $qb = $this->db->getQueryBuilder(); + $qb->select($qb->func()->count('id')) + ->from($this->getTableName()); + $result = $qb->executeQuery(); + $count = (int)$result->fetchOne(); + $result->closeCursor(); + return $count; } } diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index 8465d1bf5..cdd48b2fb 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -85,6 +85,17 @@ class SessionMapper extends QBMapper { return $this->findEntities($qb); } + public function countAll(int $documentId): int { + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'color', 'document_id', 'last_awareness_message', 'last_contact', 'user_id', 'guest_name') + ->from($this->getTableName()) + ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))); + $result = $qb->executeQuery(); + $count = (int)$result->fetchOne(); + $result->closeCursor(); + return $count; + } + /** * @return Session[] * diff --git a/lib/Migration/ResetSessionsBeforeYjs.php b/lib/Migration/ResetSessionsBeforeYjs.php index 7e9da4769..36107628e 100644 --- a/lib/Migration/ResetSessionsBeforeYjs.php +++ b/lib/Migration/ResetSessionsBeforeYjs.php @@ -2,7 +2,6 @@ namespace OCA\Text\Migration; -use OCA\Text\Db\Document; use OCA\Text\Service\DocumentService; use OCP\IAppConfig; use OCP\Migration\IOutput; @@ -27,16 +26,9 @@ class ResetSessionsBeforeYjs implements IRepairStep { return; } - $fileIds = array_map(static function (Document $document) { - return $document->getId(); - }, $this->documentService->getAll()); - - if (!$fileIds) { - return; - } - - $output->startProgress(count($fileIds)); - foreach ($fileIds as $fileId) { + $output->startProgress($this->documentService->countAll()); + foreach ($this->documentService->getAll() as $document) { + $fileId = $document->getId(); $this->documentService->unlock($fileId); $this->documentService->resetDocument($fileId, true); $output->advance(); diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 18665591a..21a7a1020 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -444,7 +444,7 @@ class DocumentService { } } - public function getAll(): array { + public function getAll(): \Generator { return $this->documentMapper->findAll(); } @@ -658,4 +658,8 @@ class DocumentService { } catch (NoLockProviderException | PreConditionNotMetException | NotFoundException $e) { } } + + public function countAll(): int { + return $this->documentMapper->countAll(); + } } diff --git a/lib/Service/SessionService.php b/lib/Service/SessionService.php index 744b763cb..f0663b057 100644 --- a/lib/Service/SessionService.php +++ b/lib/Service/SessionService.php @@ -123,6 +123,10 @@ class SessionService { }, $sessions); } + public function countAllSessions(int $documentId): int { + return $this->sessionMapper->countAll($documentId); + } + public function getActiveSessions(int $documentId): array { $sessions = $this->sessionMapper->findAllActive($documentId); return array_map(function (Session $session) {