From 399a1b238f33d9d5cacd8aa080ff0184a9db7296 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 19 Jul 2026 07:25:10 -0400 Subject: [PATCH 1/2] Add an index to speed up sorting on sqlite. --- resources/schema/sqlite/baseline.sql | 3 ++- tests/performance/Config.php | 2 +- tests/performance/Workload/Worker.php | 19 +++++++++++++++++++ tests/performance/Workload/WorkloadMix.php | 1 + 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/resources/schema/sqlite/baseline.sql b/resources/schema/sqlite/baseline.sql index ed13c780..a14f4246 100644 --- a/resources/schema/sqlite/baseline.sql +++ b/resources/schema/sqlite/baseline.sql @@ -17,7 +17,8 @@ CREATE TABLE IF NOT EXISTS entry_attribute_values ( CREATE INDEX IF NOT EXISTS idx_eav_attr_value ON entry_attribute_values (attr_name_lower, value_lower); -CREATE INDEX IF NOT EXISTS idx_eav_entry ON entry_attribute_values (entry_lc_dn); +-- Covering (entry_lc_dn leads): entry-scoped lookups plus index-only sort MIN() and correlated EXISTS. +CREATE INDEX IF NOT EXISTS idx_eav_entry ON entry_attribute_values (entry_lc_dn, attr_name_lower, value_lower); CREATE TABLE IF NOT EXISTS ldap_change_journal ( seq INTEGER NOT NULL PRIMARY KEY, diff --git a/tests/performance/Config.php b/tests/performance/Config.php index 5d33718c..6f55f1f1 100644 --- a/tests/performance/Config.php +++ b/tests/performance/Config.php @@ -54,7 +54,7 @@ final class Config 'json', ]; - public const DEFAULT_MIX = 'bind=5,search-read=50,search-eq=25,search-sub=10,search-list=5,add=2,modify=2,delete=1'; + public const DEFAULT_MIX = 'bind=5,search-read=50,search-eq=25,search-sub=10,search-list=5,search-sort=3,add=2,modify=2,delete=1'; public const DEFAULT_BIND_DN = 'cn=user,dc=foo,dc=bar'; diff --git a/tests/performance/Workload/Worker.php b/tests/performance/Workload/Worker.php index 35a55686..183cc8b5 100644 --- a/tests/performance/Workload/Worker.php +++ b/tests/performance/Workload/Worker.php @@ -14,6 +14,7 @@ namespace Tests\Performance\FreeDSx\Ldap\Workload; use FreeDSx\Ldap\ClientOptions; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; @@ -180,6 +181,7 @@ private function dispatch(LdapClient $client, string $op): void 'search-list' => $this->doSearchList($client), 'search-and' => $this->doSearchAnd($client), 'search-or' => $this->doSearchOr($client), + 'search-sort' => $this->doSearchSort($client), 'compare' => $this->doCompare($client), 'add' => $this->doAdd($client), 'modify' => $this->doModify($client), @@ -313,6 +315,23 @@ private function doSearchOr(LdapClient $client): void ); } + /** + * Subtree search with a server-side sort; the sort key is resolved per candidate from the sidecar. + */ + private function doSearchSort(LdapClient $client): void + { + $filter = $this->config->seedEntries > 0 + ? Filters::startsWith('cn', 'seed-') + : Filters::startsWith('cn', ''); + + $client->search( + $this->newSearch($filter) + ->base($this->config->baseDn) + ->useSubtreeScope(), + Controls::sort('sn'), + ); + } + /** * Builds a search request, applying the configured attribute selection so the return path can be varied. */ diff --git a/tests/performance/Workload/WorkloadMix.php b/tests/performance/Workload/WorkloadMix.php index 4c9173e8..e69d8d2c 100644 --- a/tests/performance/Workload/WorkloadMix.php +++ b/tests/performance/Workload/WorkloadMix.php @@ -31,6 +31,7 @@ final class WorkloadMix 'search-list', 'search-and', 'search-or', + 'search-sort', 'compare', 'add', 'modify', From a148b2f75c653cc1a4606b1cd6c58bb1d2e159fd Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 19 Jul 2026 08:33:56 -0400 Subject: [PATCH 2/2] Fix MySQL server-side sort to evaluate the sort key once per row instead of twice. Additionally, make CI thresholds for search-sort more realistic / sane. Allow per op p99 overrides so we can keep serach-sort in the default mix. --- .../Adapter/Dialect/PdoDialectTrait.php | 56 +++++++++++++------ .../Dialect/PdoEntryDialectInterface.php | 16 ++++-- .../Storage/Adapter/Dialect/SortKeySpec.php | 27 +++++++++ .../{SortClause.php => SortedQuery.php} | 4 +- .../Storage/Adapter/Dialect/SqliteDialect.php | 38 ++++++++----- .../Adapter/Pdo/PdoListQueryBuilder.php | 34 +++++------ tests/performance/Config.php | 7 +++ tests/performance/LoadTestCommand.php | 8 +++ tests/performance/Threshold/CiThresholds.php | 3 + .../Threshold/ThresholdEvaluator.php | 37 ++++++++---- tests/performance/Threshold/ThresholdSet.php | 11 +++- tests/performance/Workload/Worker.php | 16 ++++-- .../Adapter/Pdo/PdoListQueryBuilderTest.php | 39 ++++++++++--- 13 files changed, 217 insertions(+), 79 deletions(-) create mode 100644 src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortKeySpec.php rename src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/{SortClause.php => SortedQuery.php} (82%) diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/PdoDialectTrait.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/PdoDialectTrait.php index b0eb930b..d1f42f40 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/PdoDialectTrait.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/PdoDialectTrait.php @@ -139,24 +139,48 @@ public function querySidecarInsertPrefix(): string return 'INSERT INTO entry_attribute_values (entry_lc_dn, attr_name_lower, value_lower, value_original) VALUES '; } - public function sortKeyClause( - string $attributeLower, - string $direction, - ): SortClause { - $expr = << $sortKey) { + $alias = '__sk' . $index; + $projections[] = <<attributeLower; + } + + $projection = implode(",\n", $projections); + $order = implode(', ', $orderTerms); + $sql = << $baseParams + * @param list $sortKeys + */ + public function sortedQuery( + string $baseSql, + array $baseParams, + array $sortKeys, + ): SortedQuery; } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortKeySpec.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortKeySpec.php new file mode 100644 index 00000000..0c122551 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortKeySpec.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect; + +/** + * One resolved sort key: the lowercased attribute and its SQL direction. + * + * @author Chad Sikorra + */ +final readonly class SortKeySpec +{ + public function __construct( + public string $attributeLower, + public string $direction, + ) {} +} diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortClause.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortedQuery.php similarity index 82% rename from src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortClause.php rename to src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortedQuery.php index 589fac16..058c76a2 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortClause.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SortedQuery.php @@ -14,11 +14,11 @@ namespace FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect; /** - * ORDER BY term for one sort key plus its bound parameters. + * A base entry query rewritten with an ORDER BY, plus its parameters in bind order. * * @author Chad Sikorra */ -final readonly class SortClause +final readonly class SortedQuery { /** * @param list $params diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SqliteDialect.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SqliteDialect.php index f0c209cf..11d2ffec 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SqliteDialect.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/SqliteDialect.php @@ -74,23 +74,35 @@ public function maxDnLength(): ?int return null; } - public function sortKeyClause( - string $attributeLower, - string $direction, - ): SortClause { - // RFC 2891 §2.2: NULL is the largest value, so missing entries sort last (ASC) or first (DESC). - $nulls = $direction === 'ASC' - ? 'NULLS LAST' - : 'NULLS FIRST'; + public function sortedQuery( + string $baseSql, + array $baseParams, + array $sortKeys, + ): SortedQuery { + $terms = []; + $sortParams = []; - return new SortClause( - <<direction === 'ASC' + ? 'NULLS LAST' + : 'NULLS FIRST'; + $terms[] = <<direction} {$nulls} + SQL; + $sortParams[] = $sortKey->attributeLower; + } + + return new SortedQuery( + $baseSql . ' ORDER BY ' . implode(', ', $terms), + array_merge( + $baseParams, + $sortParams, + ), ); } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php index 1d5540fb..c34954c6 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php @@ -15,6 +15,7 @@ use FreeDSx\Ldap\Control\Sorting\SortKey; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect\PdoDialectInterface; +use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect\SortKeySpec; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\SqlFilter\SqlFilterResult; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\SqlFilter\SqlFilterUtility; @@ -58,7 +59,7 @@ public function build( }; if ($sortKeys !== []) { - $query = $this->appendSortClause( + $query = $this->applySort( $query, $sortKeys, ); @@ -218,28 +219,27 @@ private function buildFilteredSubtreeQuery( /** * @param SortKey[] $sortKeys */ - private function appendSortClause( + private function applySort( SqlQuery $query, array $sortKeys, ): SqlQuery { - $clauses = []; - $params = []; - - foreach ($sortKeys as $sortKey) { - $clause = $this->dialect->sortKeyClause( + $specs = array_values(array_map( + static fn(SortKey $sortKey): SortKeySpec => new SortKeySpec( strtolower($sortKey->getAttribute()), $sortKey->getUseReverseOrder() ? 'DESC' : 'ASC', - ); - $clauses[] = $clause->sql; - $params = array_merge( - $params, - $clause->params, - ); - } + ), + $sortKeys, + )); - return $query->appending( - ' ORDER BY ' . implode(', ', $clauses), - $params, + $sorted = $this->dialect->sortedQuery( + $query->sql, + $query->params, + $specs, + ); + + return new SqlQuery( + $sorted->sql, + $sorted->params, ); } } diff --git a/tests/performance/Config.php b/tests/performance/Config.php index 6f55f1f1..7dc6a915 100644 --- a/tests/performance/Config.php +++ b/tests/performance/Config.php @@ -66,6 +66,11 @@ final class Config public const DEFAULT_SEARCH_SUB_SIZE_LIMIT = 500; + /** + * A sorted search models a bounded display page typically, so give it a realistic limit. + */ + public const DEFAULT_SEARCH_SORT_SIZE_LIMIT = 50; + /** * Mirrors the realistic ServerOptions default so load tests run bounded rather than with lookthrough disabled. */ @@ -91,6 +96,7 @@ public function __construct( public readonly string $writeBase = self::DEFAULT_WRITE_BASE, public readonly bool $jit = true, public readonly int $searchSubSizeLimit = self::DEFAULT_SEARCH_SUB_SIZE_LIMIT, + public readonly int $searchSortSizeLimit = self::DEFAULT_SEARCH_SORT_SIZE_LIMIT, public readonly bool $monitor = true, public readonly ?string $searchAttributes = null, public readonly bool $attributesOnly = false, @@ -106,6 +112,7 @@ public function __construct( $this->assertNonNegative('warmup', $warmup); $this->assertNonNegative('seed-entries', $seedEntries); $this->assertNonNegative('search-sub-size-limit', $searchSubSizeLimit); + $this->assertNonNegative('search-sort-size-limit', $searchSortSizeLimit); $this->assertNonNegative('seed-attributes', $seedAttributes); $this->assertNonNegative('max-search-lookthrough', $maxSearchLookthrough); diff --git a/tests/performance/LoadTestCommand.php b/tests/performance/LoadTestCommand.php index ee623834..4c0f1941 100644 --- a/tests/performance/LoadTestCommand.php +++ b/tests/performance/LoadTestCommand.php @@ -214,6 +214,13 @@ protected function configure(): void 'Per-request size limit applied to search-sub ops (0 = unlimited)', (string) Config::DEFAULT_SEARCH_SUB_SIZE_LIMIT, ) + ->addOption( + 'search-sort-size-limit', + null, + InputOption::VALUE_REQUIRED, + 'Per-request size limit applied to search-sort ops (0 = unlimited)', + (string) Config::DEFAULT_SEARCH_SORT_SIZE_LIMIT, + ) ->addOption( 'search-attributes', null, @@ -445,6 +452,7 @@ private function buildConfig(InputInterface $input): Config // Swoole keeps it. --no-jit forces it off either way. jit: !(bool) $input->getOption('no-jit') && $runner !== 'pcntl', searchSubSizeLimit: $this->requireInt($input, 'search-sub-size-limit'), + searchSortSizeLimit: $this->requireInt($input, 'search-sort-size-limit'), monitor: (bool) $input->getOption('monitor'), searchAttributes: $this->optionalString($input, 'search-attributes'), attributesOnly: (bool) $input->getOption('attributes-only'), diff --git a/tests/performance/Threshold/CiThresholds.php b/tests/performance/Threshold/CiThresholds.php index 0c797036..c1c300ec 100644 --- a/tests/performance/Threshold/CiThresholds.php +++ b/tests/performance/Threshold/CiThresholds.php @@ -65,11 +65,14 @@ public static function forProfile(string $key): ThresholdSet maxErrors: 0, minThroughput: 720.0, maxP99Ms: 200.0, + // Server-sides sort file-sorts + materializes per query, so it's going to be slower. + perOpMaxP99Ms: ['search-sort' => 250.0], ), 'mysql:swoole' => new ThresholdSet( maxErrors: 0, minThroughput: 720.0, maxP99Ms: 200.0, + perOpMaxP99Ms: ['search-sort' => 250.0], ), default => throw new InvalidArgumentException(sprintf( 'Unknown CI profile "%s". Known: %s.', diff --git a/tests/performance/Threshold/ThresholdEvaluator.php b/tests/performance/Threshold/ThresholdEvaluator.php index b5c365cc..1ef581f9 100644 --- a/tests/performance/Threshold/ThresholdEvaluator.php +++ b/tests/performance/Threshold/ThresholdEvaluator.php @@ -47,10 +47,11 @@ public function evaluate( ); } - if ($thresholds->maxP99Ms !== null) { + if ($thresholds->maxP99Ms !== null || $thresholds->perOpMaxP99Ms !== []) { $gates[] = $this->evaluateMaxP99( $snapshot, $thresholds->maxP99Ms, + $thresholds->perOpMaxP99Ms, ); } @@ -122,32 +123,48 @@ private function evaluateMinThroughput( } /** + * Gates each op against its own ceiling (per-op override, else the global), reporting the op nearest its limit. + * + * @param array $perOpMaxMs + * * @return array{gate: string, passed: bool, expected: string, actual: string} */ private function evaluateMaxP99( StatsSnapshot $snapshot, - float $maxMs, + ?float $globalMaxMs, + array $perOpMaxMs, ): array { $worstOp = null; - $worstP99Ns = 0; + $worstRatio = 0.0; + $worstP99Ms = 0.0; + $worstCeilingMs = 0.0; foreach ($snapshot->operations() as $op) { - $p99 = $snapshot->latencyStats($op)['p99']; + $ceilingMs = $perOpMaxMs[$op] ?? $globalMaxMs; - if ($p99 <= $worstP99Ns) { + if ($ceilingMs === null) { continue; } - $worstP99Ns = $p99; + $p99Ms = $snapshot->latencyStats($op)['p99'] / 1_000_000; + $ratio = $p99Ms / $ceilingMs; + + if ($ratio <= $worstRatio) { + continue; + } + + $worstRatio = $ratio; $worstOp = $op; + $worstP99Ms = $p99Ms; + $worstCeilingMs = $ceilingMs; } - $worstP99Ms = $worstP99Ns / 1_000_000; - return [ 'gate' => 'max-p99-ms', - 'passed' => $worstP99Ms <= $maxMs, - 'expected' => sprintf('<= %.2f ms', $maxMs), + 'passed' => $worstOp === null || $worstP99Ms <= $worstCeilingMs, + 'expected' => $worstOp !== null + ? sprintf('<= %.2f ms (%s)', $worstCeilingMs, $worstOp) + : 'no gated ops', 'actual' => $worstOp !== null ? sprintf('%.2f ms (%s)', $worstP99Ms, $worstOp) : 'no data', diff --git a/tests/performance/Threshold/ThresholdSet.php b/tests/performance/Threshold/ThresholdSet.php index 9e4a299d..f44d3c3d 100644 --- a/tests/performance/Threshold/ThresholdSet.php +++ b/tests/performance/Threshold/ThresholdSet.php @@ -18,11 +18,15 @@ */ final class ThresholdSet { + /** + * @param array $perOpMaxP99Ms per-op p99 ceilings (ms) that override maxP99Ms for the named op + */ public function __construct( public readonly ?float $maxErrorRate = null, public readonly ?int $maxErrors = null, public readonly ?float $minThroughput = null, public readonly ?float $maxP99Ms = null, + public readonly array $perOpMaxP99Ms = [], ) {} public function isEmpty(): bool @@ -30,7 +34,8 @@ public function isEmpty(): bool return $this->maxErrorRate === null && $this->maxErrors === null && $this->minThroughput === null - && $this->maxP99Ms === null; + && $this->maxP99Ms === null + && $this->perOpMaxP99Ms === []; } /** @@ -43,6 +48,10 @@ public function withOverrides(self $overrides): self maxErrors: $overrides->maxErrors ?? $this->maxErrors, minThroughput: $overrides->minThroughput ?? $this->minThroughput, maxP99Ms: $overrides->maxP99Ms ?? $this->maxP99Ms, + perOpMaxP99Ms: array_merge( + $this->perOpMaxP99Ms, + $overrides->perOpMaxP99Ms, + ), ); } } diff --git a/tests/performance/Workload/Worker.php b/tests/performance/Workload/Worker.php index 183cc8b5..ecb99917 100644 --- a/tests/performance/Workload/Worker.php +++ b/tests/performance/Workload/Worker.php @@ -316,18 +316,24 @@ private function doSearchOr(LdapClient $client): void } /** - * Subtree search with a server-side sort; the sort key is resolved per candidate from the sidecar. + * Server-side sort over a selective subset (a realistic sorted search filters first), size-limited to a display page. */ private function doSearchSort(LdapClient $client): void { $filter = $this->config->seedEntries > 0 - ? Filters::startsWith('cn', 'seed-') + ? Filters::startsWith('cn', 'seed-1') : Filters::startsWith('cn', ''); + $request = $this->newSearch($filter) + ->base($this->config->baseDn) + ->useSubtreeScope(); + + if ($this->config->searchSortSizeLimit > 0) { + $request->sizeLimit($this->config->searchSortSizeLimit); + } + $client->search( - $this->newSearch($filter) - ->base($this->config->baseDn) - ->useSubtreeScope(), + $request, Controls::sort('sn'), ); } diff --git a/tests/unit/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilderTest.php b/tests/unit/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilderTest.php index c594e2cb..8c6296cd 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilderTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilderTest.php @@ -58,7 +58,7 @@ public function test_sqlite_descending_orders_nulls_first_with_one_param(): void self::assertSame(['cn'], $params); } - public function test_mysql_ascending_emulates_nulls_last_with_two_params(): void + public function test_mysql_ascending_projects_the_key_once_and_orders_nulls_last(): void { [$sql, $params] = $this->rootQuery( new PdoListQueryBuilder(new MysqlDialect()), @@ -66,11 +66,12 @@ public function test_mysql_ascending_emulates_nulls_last_with_two_params(): void SortKey::ascending('cn'), ); - self::assertStringContainsString('IS NULL ASC', $sql); - self::assertSame(['cn', 'cn'], $params); + self::assertStringContainsString('AS __sk0', $sql); + self::assertStringContainsString('ORDER BY __sk0 IS NULL ASC, __sk0 ASC', $sql); + self::assertSame(['cn'], $params); } - public function test_mysql_descending_emulates_nulls_first_with_two_params(): void + public function test_mysql_descending_projects_the_key_once_and_orders_nulls_first(): void { [$sql, $params] = $this->rootQuery( new PdoListQueryBuilder(new MysqlDialect()), @@ -78,21 +79,41 @@ public function test_mysql_descending_emulates_nulls_first_with_two_params(): vo SortKey::descending('cn'), ); - self::assertStringContainsString('IS NULL DESC', $sql); - self::assertSame(['cn', 'cn'], $params); + self::assertStringContainsString('ORDER BY __sk0 IS NULL DESC, __sk0 DESC', $sql); + self::assertSame(['cn'], $params); } - public function test_mysql_multi_key_preserves_param_order(): void + public function test_mysql_multi_key_projects_each_key_and_orders_in_sequence(): void { - [, $params] = $this->rootQuery( + [$sql, $params] = $this->rootQuery( new PdoListQueryBuilder(new MysqlDialect()), null, SortKey::ascending('sn'), SortKey::descending('cn'), ); + self::assertStringContainsString( + 'ORDER BY __sk0 IS NULL ASC, __sk0 ASC, __sk1 IS NULL DESC, __sk1 DESC', + $sql, + ); + self::assertSame( + ['sn', 'cn'], + $params, + ); + } + + public function test_mysql_sort_params_bind_before_the_base_query_params(): void + { + [$sql, $params] = $this->rootQuery( + new PdoListQueryBuilder(new MysqlDialect()), + new SqlFilterResult('eav.value_lower = ?', ['smith']), + SortKey::ascending('sn'), + ); + + // The projected sort key precedes the nested base query, so its param binds first. + self::assertStringContainsString('FROM (', $sql); self::assertSame( - ['sn', 'sn', 'cn', 'cn'], + ['sn', 'smith'], $params, ); }