Skip to content

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 15, 2023

Improvements are split into individual commits for easier review.

Overall this adds additional caching layers and groups queries where we performed them for n results into one.

ToDo

  • Get in Optimise upcomming overview creation #3793 first / might fix OverviewService to perform queries per board
  • StackMapper::find could use some caching on request level
  • BoardService::find does not need full details (users/settings/sessions) when we set the relative board on cards
  • Group queries for related stacks
  • Make sure that in queries do not exceed the oracle limit: More than 1000 expressions in a list are not allowed on Oracle.

Follow up issues

  • CardMapper findBoardId cache should probably use something like WithLocalCache to avoid heavy round trips to redis (which happens at least once for every file share)
  • Enrichment of board data could be grouped queries (might only have a noticeable effect on hundreds of boards a user has)

Board list

  • Removes 1 query per board the user has access to
  • Removes 1 additional query

Screenshot 2023-02-17 at 09 50 43

Overview

  • Removes 2-4 queries per card that is listed in the overview (extra queries still needed if a card has comments to fetch the unread counter)

Screenshot 2023-02-17 at 09 51 52

Single board request

  • removes 1 query for each sharing rule that a board has

Screenshot 2023-02-17 at 09 53 45

Stack list

  • removes 2-4 queries per card that is on the board
  • reduce query to fetch stack details from the amount of cards to the number of stacks (40 cards, 4 stacks -> 40 to 4 queries)

Screenshot 2023-02-17 at 09 55 15

Propfind on Deck folder (3200 incoming deck file shares)

Screenshot 2023-02-17 at 09 59 12

Testing

Script to generate random board content
<?php
/*
 * @copyright Copyright (c) 2021 Julius Härtl <jus@bitgrid.net>
 *
 * @author Julius Härtl <jus@bitgrid.net>
 *
 * @license GNU AGPL version 3 or any later version
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License as
 * published by the Free Software Foundation, either version 3 of the
 * License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License
 * along with this program. If not, see <http://www.gnu.org/licenses/>.
 *
 */

declare(strict_types=1);

if ($argc === 1) {
	echo 'Usage: php deck-random.php [userId] [boardCount] [stackCount] [cardCount]';
	exit;
}

require_once __DIR__ . '/../lib/versioncheck.php';

function generateRandomString($length = 10) {
	$characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
	$charactersLength = strlen($characters);
	$randomString = '';
	for ($i = 0; $i < $length; $i++) {
		$randomString .= $characters[random_int(0, $charactersLength - 1)];
	}
	return $randomString;
}

function random_color_part() {
	return str_pad(dechex(mt_rand(0, 255)), 2, '0', STR_PAD_LEFT);
}

function random_color() {
	return random_color_part() . random_color_part() . random_color_part();
}

function randomDateInRange(DateTime $start, DateTime $end) {
	$randomTimestamp = mt_rand($start->getTimestamp(), $end->getTimestamp());
	$randomDate = new DateTime();
	$randomDate->setTimestamp($randomTimestamp);
	return $randomDate;
}

try {
	require_once __DIR__ . '/../lib/base.php';
	OC_App::loadApps(['authentication']);
	OC_App::loadApps(['filesystem', 'logging']);
	OC_App::loadApps();

	$user = \OC::$server->getUserManager()->get($argv[1] ?? 'admin');
	$uid = $user->getUID() ?? 'admin';
	\OC::$server->getUserSession()->setUser($user);

	$boardCount = $argv[2] ?? 1;
	$stackCount = $argv[3] ?? 4;
	$cardCount = $argv[4] ?? 10;
	$attachmentCount = 10;

	/** @var \OCA\Deck\Service\BoardService $boardService */
	$boardService = \OC::$server->get(\OCA\Deck\Service\BoardService::class);
	/** @var \OCA\Deck\Service\StackService $stackService */
	$stackService = \OC::$server->get(\OCA\Deck\Service\StackService::class);
	/** @var \OCA\Deck\Service\CardService $cardService */
	$cardService = \OC::$server->get(\OCA\Deck\Service\CardService::class);

	$shareManager = \OCP\Server::get(\OCP\Share\IManager::class);

	$root = \OCP\Server::get(\OCP\Files\IRootFolder::class)->getUserFolder($uid);
	$deckFolder = $root->nodeExists('deck_random') ? $root->get('deck_random') : $root->newFolder('deck_random');
	for ($h = 0; $h < $boardCount; $h++) {
		$board = $boardService->create(generateRandomString(), $uid, random_color());
		$users = \OC::$server->getUserManager()->search('');
		$acls = array_rand($users, 5);
		$boardService->addAcl($board->getId(), \OCA\Deck\Db\Acl::PERMISSION_TYPE_USER, 'bob', true, true, true);
		foreach ($acls as $acl) {
			if ($acl !== $uid && $acl !== 'bob') {
				$boardService->addAcl($board->getId(), \OCA\Deck\Db\Acl::PERMISSION_TYPE_USER, $acl, true, true, true);
			}
		}
		for ($i = 0; $i < $stackCount; $i++) {
			echo "Created stack $i\n";
			$stack = $stackService->create(generateRandomString(), $board->getId(), -1);
			for ($j = 0; $j < $cardCount; $j++) {
				echo "Created card $i.$j\n";
				$randomDate = date("Y-m-d H:i:s", mt_rand(1262055681, 1262055681));
				$card = $cardService->create(generateRandomString(100), $stack->getId(), 'text', -1, $uid, generateRandomString(2000), null);

				for ($ai = 0; $ai < $attachmentCount; $ai++) {
					$file = $deckFolder->newFile('attachment-' . $card->getId() . '-' . $ai . '.txt', $randomDate);
					$share = $shareManager->newShare();
					$share->setSharedBy($uid);
					$share->setSharedWith((string)$card->getId());
					$share->setPermissions(1);
					$share->setShareType(\OCP\Share\IShare::TYPE_DECK);
					$share->setNode($file);
					$shareManager->createShare($share);
				}
			}
		}
	}
} catch (\Throwable $e) {
	echo $e->getMessage() . PHP_EOL;
	echo $e->getTraceAsString() . PHP_EOL;
}

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
use OCP\IUser;
use OCP\IUserManager;

class User extends RelationalObject {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love it if we could get that RelationalObject goodness into server at some point, seems very useful!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too happy anymore with the approach and think it should actually be decoupled from the entity itself. I'm more in favour of having separate Model classes in the future like Talk has.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, will have a look at that

const baseCount = parseInt(myOutput, 10)
const absoluteIncrease = queryCount - baseCount
const relativeIncrease = baseCount <= 0 ? 100 : (parseInt((absoluteIncrease / baseCount * 10000), 10) / 100)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we print these stats to stdout just for curiosity?

@marcelklehr
Copy link
Member

How much impact do these changes have? out of curiosity?

@juliusknorr
Copy link
Member Author

Added summary of the saved queries to the first post above each blackfire screenshot ;)

@juliusknorr juliusknorr merged commit 15c1c9d into main Feb 20, 2023
@delete-merged-branch delete-merged-branch bot deleted the enh/perf branch February 20, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants