Performance of `getRooms`

[SystemKeeper]

Just some things I noticed while logging all DB queries on getRooms call, feel free to close.

☑️ Last message is queried individually

Every last message is queried individually, like this:

DB QueryBuilder: 'SELECT * FROM `*PREFIX*comments` WHERE `id` = :id' with parameters: id => '990'
DB QueryBuilder: 'SELECT * FROM `*PREFIX*comments` WHERE `id` = :id' with parameters: id => '997'
...

Can we load these messages in a single query?

☑️ Preload shares and files metadata

While the latest changes significantly improved performance with not setting up the FS, for every last message that is a share, we query the share and the files metadata:

DB QueryBuilder: 'SELECT `s`.*, `f`.`fileid`, `f`.`path`, `f`.`permissions` as `f_permissions`, `f`.`storage`, `f`.`path_hash`, `f`.`parent` as `f_parent`, `f`.`name`, `f`.`mimetype`, `f`.`mimepart`, `f`.`size`, `f`.`mtime`, `f`.`storage_mtime`, `f`.`encrypted`, `f`.`unencrypted_size`, `f`.`etag`, `f`.`checksum`, `st`.`id` AS `storage_string_id` FROM `*PREFIX*share` `s` LEFT JOIN `*PREFIX*filecache` `f` ON `s`.`file_source` = `f`.`fileid` LEFT JOIN `*PREFIX*storages` `st` ON `f`.`storage` = `st`.`numeric_id` WHERE (`s`.`id` IN (:dcValue1)) AND (`s`.`share_type` = :dcValue2)' with parameters: dcValue1 => ('18'), dcValue2 => '10'
DB QueryBuilder: 'SELECT `json`, `sync_token` FROM `*PREFIX*files_metadata` WHERE `file_id` = :dcValue1' with parameters: dcValue1 => '847'

We already have a perloadShares method for messages, which loads the shares and the files metdata. Can we do the same here ?

spreed/lib/Controller/ChatController.php

Line 348 in 3afc9c8

protected function preloadShares(array $comments): void {

☑️ Addressbook queried

DB QueryBuilder: 'SELECT * FROM `*PREFIX*dav_cal_proxy` WHERE `proxy_id` = :dcValue1' with parameters: dcValue1 => 'principals/users/admin'
DB QueryBuilder: 'SELECT * FROM `*PREFIX*properties` WHERE (`userid` = :dcValue1) AND (`propertypath` = :dcValue2) AND (`propertyname` = :dcValue3)' with parameters: dcValue1 => 'admin', dcValue2 => 'addressbooks/users/admin/contacts', dcValue3 => '{http://owncloud.org/ns}enabled'
DB QueryBuilder: 'SELECT * FROM `*PREFIX*properties` WHERE (`userid` = :dcValue1) AND (`propertypath` = :dcValue2) AND (`propertyname` = :dcValue3)' with parameters: dcValue1 => 'admin', dcValue2 => 'addressbooks/users/admin/z-server-generated--system', dcValue3 => '{http://owncloud.org/ns}enabled'

DB QueryBuilder: 'SELECT `a`.`id`, `a`.`uri`, `a`.`displayname`, `a`.`principaluri`, `a`.`description`, `a`.`synctoken`, `s`.`access` FROM `*PREFIX*dav_shares` `s` INNER JOIN `*PREFIX*addressbooks` `a` ON `s`.`resourceid` = `a`.`id` WHERE (`s`.`principaluri` IN (:dcValue3)) AND (`s`.`type` = :dcValue4) AND (`s`.`id` NOT IN (SELECT `id` FROM `*PREFIX*dav_shares` `d` WHERE (`d`.`access` = :dcValue1) AND (`d`.`principaluri` IN (:dcValue2))))' with parameters: dcValue1 => '5', dcValue2 => ('principals/groups/admin', 'principals/groups/admin+group', 'principals/circles/emqUs9WPLPxNNVqbCDapOjfyxhMr7bZ', 'principals/users/admin'), dcValue3 => ('principals/groups/admin', 'principals/groups/admin+group', 'principals/circles/emqUs9WPLPxNNVqbCDapOjfyxhMr7bZ', 'principals/users/admin'), dcValue4 => 'addressbook'

I first thought this is because of the user status, but why do we need to query all of our own addressbooks and also the server generated one?

☑️ Guest attendee is queried when last message is system message

Lots of:

DB QueryBuilder: 'SELECT `a`.`room_id`, `a`.`actor_type`, `a`.`actor_id`, `a`.`display_name`, `a`.`pin`, `a`.`participant_type`, `a`.`favorite`, `a`.`notification_level`, `a`.`notification_calls`, `a`.`last_joined_call`, `a`.`last_read_message`, `a`.`last_mention_message`, `a`.`last_mention_direct`, `a`.`read_privacy`, `a`.`permissions`, `a`.`access_token`, `a`.`remote_id`, `a`.`invited_cloud_id`, `a`.`phone_number`, `a`.`call_id`, `a`.`state`, `a`.`unread_messages`, `a`.`last_attendee_activity`, `a`.`archived`, `a`.`id` AS `a_id` FROM `*PREFIX*talk_attendees` `a` WHERE (`a`.`actor_type` = :dcValue1) AND (`a`.`actor_id` = :dcValue2) AND (`a`.`room_id` = :dcValue3) LIMIT 1' with parameters: dcValue1 => 'guests', dcValue2 => 'system', dcValue3 => '127'

I assume it happens because system messages have actorId = system and actorType = guests? From a quick test, the query comes from

spreed/lib/Chat/Parser/SystemMessage.php

Lines 1082 to 1099 in 3afc9c8

	protected function getGuestName(Room $room, string $actorType, string $actorId): string {
	if ($actorId === Attendee::ACTOR_ID_CLI) {
	return $this->l->t('Guest');
	}
	try {
	$participant = $this->participantService->getParticipantByActor($room, $actorType, $actorId);
	$name = $participant->getAttendee()->getDisplayName();
	if ($name === '' && $actorType === Attendee::ACTOR_EMAILS) {
	$name = $actorId;
	} elseif ($name === '') {
	return $this->l->t('Guest');
	}
	return $this->l->t('%s (guest)', [$name]);
	} catch (ParticipantNotFoundException $e) {
	return $this->l->t('Guest');
	}
	}

So should we handle ACTOR_ID_SYSTEM in the same way as ACTOR_ID_CLI?

[nickvergessen]

Last message is queried individually

Only if preloading failed

[nickvergessen]

Guest attendee is queried when last message is system message

• Fixing in test: Run scaling tests with query.log #14812 atm will extract before merge

[nickvergessen]

Last message is queried individually

• fix(chat): Fix preloading the last chat message with the room query #14808

Guest attendee is queried when last message is system message

• fix(chat): Don't look for some more hardcoded guests #14814

[nickvergessen]

Addressbook queried

• fix(federation): Don't load the addressbook when resolving a cloud ID server#52066

[nickvergessen]

Preload shares and files metadata

• fix(performance): Improve performance with many file shares as last m… #14829