Skip to content

Commit da356e9

Browse files
committed
Perf: Optimize shares loading by eliminating N+1 queries (code review fixes)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent 938b7c8 commit da356e9

4 files changed

Lines changed: 43 additions & 30 deletions

File tree

lib/Db/ShareMapper.php

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,25 @@ public function findShareForNode(int $nodeId, string $nodeType, string $receiver
100100
* @throws Exception
101101
*/
102102
public function findAllSharesFor(string $nodeType, array $receivers, string $userId, ?string $receiverType = 'user'): array {
103-
$qb = $this->db->getQueryBuilder();
104-
$qb->select('*')
105-
->from($this->table)
106-
->where($qb->expr()->in('receiver', $qb->createNamedParameter($receivers, IQueryBuilder::PARAM_STR_ARRAY)))
107-
->andWhere($qb->expr()->neq('sender', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)))
108-
->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType, IQueryBuilder::PARAM_STR)))
109-
->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter($receiverType, IQueryBuilder::PARAM_STR)));
110-
return $this->findEntities($qb);
103+
if (!$receivers) {
104+
return [];
105+
}
106+
107+
$chunks = [];
108+
// deduct extra parameters (sender, node type, receiver type)
109+
foreach (array_chunk($receivers, 1000 - 3) as $receiversChunk) {
110+
$qb = $this->db->getQueryBuilder();
111+
$qb->select('*')
112+
->from($this->table)
113+
->where($qb->expr()->in('receiver', $qb->createNamedParameter($receiversChunk, IQueryBuilder::PARAM_STR_ARRAY)))
114+
->andWhere($qb->expr()->neq('sender', $qb->createNamedParameter($userId)))
115+
->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType)))
116+
->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter($receiverType)));
117+
118+
$chunks[] = $this->findEntities($qb);
119+
}
120+
121+
return array_merge(...$chunks);
111122
}
112123

113124
/**

lib/Db/TableMapper.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,18 @@ public function findMany(array $ids): array {
7070
return $result;
7171
}
7272

73-
$qb = $this->db->getQueryBuilder();
74-
$qb->select('*')
75-
->from($this->table)
76-
->where($qb->expr()->in('id', $qb->createNamedParameter(array_keys($missing), IQueryBuilder::PARAM_INT_ARRAY)));
73+
$missing = array_keys($missing);
74+
foreach (array_chunk($missing, 1000) as $missingChunk) {
75+
$qb = $this->db->getQueryBuilder();
76+
$qb->select('*')
77+
->from($this->table)
78+
->where($qb->expr()->in('id', $qb->createNamedParameter($missingChunk, IQueryBuilder::PARAM_INT_ARRAY)));
7779

78-
$entities = $this->findEntities($qb);
79-
foreach ($entities as $entity) {
80-
$id = $entity->getId();
81-
$this->cache[(string)$id] = $entity;
82-
$result[$id] = $entity;
80+
foreach ($this->findEntities($qb) as $entity) {
81+
$id = $entity->getId();
82+
$this->cache[(string)$id] = $entity;
83+
$result[$id] = $entity;
84+
}
8385
}
8486
return $result;
8587
}

lib/Db/ViewMapper.php

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,17 +70,19 @@ public function findMany(array $ids): array {
7070
return $result;
7171
}
7272

73-
$qb = $this->db->getQueryBuilder();
74-
$qb->select('v.*', 't.ownership')
75-
->from($this->table, 'v')
76-
->innerJoin('v', 'tables_tables', 't', 't.id = v.table_id')
77-
->where($qb->expr()->in('v.id', $qb->createNamedParameter(array_keys($missing), IQueryBuilder::PARAM_INT_ARRAY)));
78-
79-
$entities = $this->findEntities($qb);
80-
foreach ($entities as $entity) {
81-
$id = $entity->getId();
82-
$this->cache[(string)$id] = $entity;
83-
$result[$id] = $entity;
73+
$missing = array_keys($missing);
74+
foreach (array_chunk($missing, 1000) as $missingChunk) {
75+
$qb = $this->db->getQueryBuilder();
76+
$qb->select('v.*', 't.ownership')
77+
->from($this->table, 'v')
78+
->innerJoin('v', 'tables_tables', 't', 't.id = v.table_id')
79+
->where($qb->expr()->in('v.id', $qb->createNamedParameter($missingChunk, IQueryBuilder::PARAM_INT_ARRAY)));
80+
81+
foreach ($this->findEntities($qb) as $entity) {
82+
$id = $entity->getId();
83+
$this->cache[(string)$id] = $entity;
84+
$result[$id] = $entity;
85+
}
8486
}
8587
return $result;
8688
}

lib/Helper/CircleHelper.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use OCA\Circles\Model\Circle;
1212
use OCA\Circles\Model\Member;
1313
use OCA\Circles\Model\Probes\CircleProbe;
14-
use OCA\Tables\Errors\InternalError;
1514
use OCP\App\IAppManager;
1615
use OCP\IL10N;
1716
use OCP\Server;
@@ -77,7 +76,6 @@ public function getCircleDisplayName(string $circleId, string $userId): string {
7776
/**
7877
* @param string $userId
7978
* @return Circle[]
80-
* @throws InternalError
8179
*/
8280
public function getUserCircles(string $userId): array {
8381
if (!$this->circlesEnabled) {

0 commit comments

Comments
 (0)