From df375cdd8a3e202f524fcf374587d003b1ca7f94 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 12 Sep 2026 21:19:00 -0700 Subject: [PATCH] perf(mobile): reuse Linear issue ordering when grouping list and board (#20314) --- .../mobile-linear-group-sorted-benchmark.mjs | 97 +++++++++++++++++++ .../tasks/mobile-linear-group-sorted.test.ts | 69 +++++++++++++ .../mobile-tasks-refactor-parity.test.ts | 8 +- .../src/tasks/mobile-tasks-reviewer-linear.ts | 16 ++- ...le-tasks-provider-view-projection.test.tsx | 13 ++- ...-mobile-tasks-provider-view-projection.tsx | 10 +- 6 files changed, 199 insertions(+), 14 deletions(-) create mode 100644 config/scripts/mobile-linear-group-sorted-benchmark.mjs create mode 100644 mobile/src/tasks/mobile-linear-group-sorted.test.ts diff --git a/config/scripts/mobile-linear-group-sorted-benchmark.mjs b/config/scripts/mobile-linear-group-sorted-benchmark.mjs new file mode 100644 index 00000000000..529515a99ae --- /dev/null +++ b/config/scripts/mobile-linear-group-sorted-benchmark.mjs @@ -0,0 +1,97 @@ +import assert from 'node:assert/strict' +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' +import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs' + +const baseline = process.argv[2] +if (!baseline) { + throw new Error( + 'Usage: node config/scripts/mobile-linear-group-sorted-benchmark.mjs ' + ) +} +async function load(file, contents) { + const result = await build({ + stdin: { contents, loader: 'ts', resolveDir: dirname(resolve(file)) }, + bundle: true, + platform: 'node', + format: 'esm', + write: false, + logLevel: 'silent', + tsconfigRaw: {}, + plugins: [ + { + name: 'theme-only', + setup(bundler) { + bundler.onResolve({ filter: /mobile-tasks-dependencies$/ }, () => ({ + path: resolve('mobile/src/theme/mobile-theme.ts') + })) + } + } + ] + }) + return await import( + `data:text/javascript;base64,${Buffer.from(result.outputFiles[0].text).toString('base64')}` + ) +} +const file = 'mobile/src/tasks/mobile-tasks-reviewer-linear.ts' +const original = await load( + file, + execFileSync('git', ['show', `${baseline}:${file}`], { encoding: 'utf8', windowsHide: true }) +) +const current = await load(file, readFileSync(file, 'utf8')) +const sorterRef = process.argv[3] +const sorter = sorterRef + ? await load( + file, + execFileSync('git', ['show', `${sorterRef}:${file}`], { encoding: 'utf8', windowsHide: true }) + ) + : original +const results = [] +for (const count of [25, 200, 1000]) { + const items = Array.from({ length: count }, (_, i) => ({ + id: `item-${i}`, + identifier: `ENG-${(i * 37) % count}`, + updatedAt: new Date(1700000000000 - i * 100000).toISOString(), + priority: i % 5, + state: { name: `state-${i % 4}`, color: 'red' }, + team: { id: `team-${i % 3}`, name: 'Team' }, + assignee: null + })) + for (const order of ['identifier', 'updated', 'priority']) { + const run = (arm) => { + const sorted = sorter.sortLinearIssues + ? sorter.sortLinearIssues(items, order) + : [...items].sort((a, b) => sorter.compareLinearIssues(a, b, order)) + return arm === 'before' + ? [ + sorter.groupLinearIssues(sorted, 'none', order), + sorter.groupLinearIssues(sorted, 'status', order) + ] + : [ + current.groupSortedLinearIssues(sorted, 'none'), + current.groupSortedLinearIssues(sorted, 'status') + ] + } + assert.deepEqual(run('after'), run('before')) + for (let i = 0; i < 10; i++) { + run('before') + run('after') + } + const samples = { before: [], after: [] } + for (const pair of buildCounterbalancedSchedule(8, 'before', 'after')) { + for (const arm of pair) { + global.gc?.() + const start = performance.now() + for (let i = 0; i < 10; i++) { + run(arm) + } + samples[arm].push((performance.now() - start) / 10) + } + } + results.push({ count, order, samples }) + } +} +console.log(JSON.stringify({ baseline, sorterRef, node: process.version, results }, null, 2)) diff --git a/mobile/src/tasks/mobile-linear-group-sorted.test.ts b/mobile/src/tasks/mobile-linear-group-sorted.test.ts new file mode 100644 index 00000000000..fbd87f9c68e --- /dev/null +++ b/mobile/src/tasks/mobile-linear-group-sorted.test.ts @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { LinearIssue } from './mobile-tasks-provider-detail-types' +import { + sortLinearIssues, + groupLinearIssues, + groupSortedLinearIssues +} from './mobile-tasks-reviewer-linear' + +vi.mock('./mobile-tasks-dependencies', () => import('../theme/mobile-theme')) +afterEach(() => vi.restoreAllMocks()) + +const issues: LinearIssue[] = Array.from({ length: 60 }, (_, i) => ({ + id: `${i}`, + identifier: ['ENG-10', 'ENG-2', 'Ä-1', 'Å-1', 'é-2', 'e\u0301-2', 'İ-3'][i % 7], + title: 'Task', + url: '', + labels: [], + priority: i % 5, + updatedAt: ['2026-02-01', 'invalid', '1970-01-01', '2026-02-01', '2025-01-01'][ + Math.floor(i / 5) % 5 + ], + state: { name: i % 2 ? 'Todo' : 'Done', type: 'started', color: '' }, + team: { id: `${i % 3}`, name: `Team ${i % 3}`, key: 'ENG' } +})) + +describe('mobile Linear grouping of sorted issues', () => { + it.each(['updated', 'identifier', 'priority'] as const)( + 'preserves %s ordering, ties and group metadata', + (order) => { + const sorted = Object.freeze(sortLinearIssues(issues, order)) + for (const group of ['none', 'status', 'assignee', 'team', 'priority'] as const) { + const expected = groupLinearIssues([...sorted], group, order) + const actual = groupSortedLinearIssues(sorted, group) + expect(actual).toEqual(expected) + actual.forEach((section, index) => { + expect(section.issues).not.toBe(sorted) + section.issues.forEach((issue, offset) => + expect(issue).toBe(expected[index].issues[offset]) + ) + }) + } + } + ) + + it('does no date parsing or collation after ordering has been established', () => { + const sorted = sortLinearIssues(issues, 'updated') + const parse = vi.spyOn(Date, 'parse') + const compare = vi.spyOn(String.prototype, 'localeCompare') + groupSortedLinearIssues(sorted, 'none') + groupSortedLinearIssues(sorted, 'status') + expect(parse).not.toHaveBeenCalled() + expect(compare).not.toHaveBeenCalled() + groupLinearIssues(sorted, 'status', 'updated') + expect(parse).toHaveBeenCalled() + }) + + it('returns independent issue arrays for empty, singleton and ungrouped inputs', () => { + for (const input of [[], [issues[0]], issues]) { + const sorted = Object.freeze([...input]) + const first = groupSortedLinearIssues(sorted, 'none') + const second = groupSortedLinearIssues(sorted, 'none') + expect(first).toEqual(second) + expect(first[0].issues).not.toBe(second[0].issues) + first[0].issues.pop() + expect(second[0].issues).toEqual(sorted) + } + expect(groupSortedLinearIssues([], 'status')).toEqual([]) + }) +}) diff --git a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts index 410e5ac1173..09fa587e0b1 100644 --- a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts +++ b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts @@ -17,10 +17,10 @@ const hash = (parts: string[] | string): string => .digest('hex') // Task and Linear sort tests cover computation changes; render/style guards remain. -const EXPECTED_SCREEN_HOOKS = '09b5710f48b0421dfaa86b622e6223df6317f097decc384b05006edfdbe4e98a' +const EXPECTED_SCREEN_HOOKS = '25c9a72805e48caa9c6758d14a128fea1bdc6cfdea3e4e39a7f0026c5933c8d7' const EXPECTED_DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' -const EXPECTED_STATEMENTS = 'd10837338d241c11e20789233744f95389773d59063d8736d8978a15e0e55a59' -const EXPECTED_DECLARATIONS = '3fb5a15c92960124ea2b9a222d8ed4786b1faab965d56a7a8896cc7ea44a6afc' +const EXPECTED_STATEMENTS = '1413f6e843f7a7ae849767b26b25d5eafc7fffda1fc37f283fbd883d9d8a4bda' +const EXPECTED_DECLARATIONS = '6ad0397123e59fc1047a14049c86ff31d81723673a7a7f5c41677471aec58415' const EXPECTED_SEMANTICS = '4758ba019e4ff7cadd7ee02338719fa4fc4e1443e34cc290842819cfa1a70181' const EXPECTED_STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' const EXPECTED_RENDER_TREE = '2111145136b1e4fbca150d4792d735a90e992488e9934cfc1a8b8f3be981f39f' @@ -44,7 +44,7 @@ describe('Mobile Tasks refactor parity', () => { it('preserves every moved top-level declaration', () => { const declarations = readMobileTasksDeclarationSignatures() - expect(declarations).toHaveLength(192) + expect(declarations).toHaveLength(194) expect(hash(declarations)).toBe(EXPECTED_DECLARATIONS) }) diff --git a/mobile/src/tasks/mobile-tasks-reviewer-linear.ts b/mobile/src/tasks/mobile-tasks-reviewer-linear.ts index 9c41c30db14..ed5e51f8bf7 100644 --- a/mobile/src/tasks/mobile-tasks-reviewer-linear.ts +++ b/mobile/src/tasks/mobile-tasks-reviewer-linear.ts @@ -139,7 +139,21 @@ export function groupLinearIssues( groupBy: LinearGroupBy, orderBy: LinearOrderBy ): LinearIssueSection[] { - const sorted = sortLinearIssues(issues, orderBy) + return groupOrderedLinearIssues(sortLinearIssues(issues, orderBy), groupBy) +} + +/** The caller must sort issues by its selected order before grouping. */ +export function groupSortedLinearIssues( + issues: readonly LinearIssue[], + groupBy: LinearGroupBy +): LinearIssueSection[] { + return groupOrderedLinearIssues([...issues], groupBy) +} + +function groupOrderedLinearIssues( + sorted: LinearIssue[], + groupBy: LinearGroupBy +): LinearIssueSection[] { if (groupBy === 'none') { return [{ key: 'all', label: 'Issues', color: colors.accentBlue, issues: sorted }] } diff --git a/mobile/src/tasks/use-mobile-tasks-provider-view-projection.test.tsx b/mobile/src/tasks/use-mobile-tasks-provider-view-projection.test.tsx index 4891db7b452..0eb8d3d32f7 100644 --- a/mobile/src/tasks/use-mobile-tasks-provider-view-projection.test.tsx +++ b/mobile/src/tasks/use-mobile-tasks-provider-view-projection.test.tsx @@ -38,6 +38,10 @@ vi.mock('./mobile-tasks-legacy-foundation', async () => { groupLinearIssues: (issues: LinearIssue[], groupBy: LinearGroupBy, orderBy: LinearOrderBy) => { groupingInputSizes.push(issues.length) return linear.groupLinearIssues(issues, groupBy, orderBy) + }, + groupSortedLinearIssues: (issues: readonly LinearIssue[], groupBy: LinearGroupBy) => { + groupingInputSizes.push(issues.length) + return linear.groupSortedLinearIssues(issues, groupBy) } } }) @@ -392,11 +396,12 @@ describe('useMobileTasksProviderViewProjection grouping work', () => { expect(after.groupingCalls).toBe(1) expect(before.issueVisits).toBe(100) expect(after.issueVisits).toBe(50) - // The dropped grouping re-sorted an already sorted 50-issue array: n-1 comparisons. - expect(before.comparisons - after.comparisons).toBe(input.items.length - 1) + // Both legacy groupings re-sorted an already sorted 50-issue array (n-1 comparisons + // each); one call is gone and the other groups without re-sorting. + expect(before.comparisons - after.comparisons).toBe(2 * (input.items.length - 1)) }) - it('leaves the none grouping work untouched', () => { + it('keeps both none grouping calls but drops their re-sorts', () => { const input = { ...DEFAULT_INPUT, linearGroupBy: 'none' as const } const before = countLinearWork(() => { legacyProjection(input) @@ -406,7 +411,7 @@ describe('useMobileTasksProviderViewProjection grouping work', () => { }) expect([before.groupingCalls, after.groupingCalls]).toEqual([2, 2]) expect([before.issueVisits, after.issueVisits]).toEqual([100, 100]) - expect(after.comparisons).toBe(before.comparisons) + expect(before.comparisons - after.comparisons).toBe(2 * (input.items.length - 1)) }) it('does no grouping work on an unrelated rerender', () => { diff --git a/mobile/src/tasks/use-mobile-tasks-provider-view-projection.tsx b/mobile/src/tasks/use-mobile-tasks-provider-view-projection.tsx index cee917564c1..aa7bae19d6b 100644 --- a/mobile/src/tasks/use-mobile-tasks-provider-view-projection.tsx +++ b/mobile/src/tasks/use-mobile-tasks-provider-view-projection.tsx @@ -16,7 +16,7 @@ import { PR_PRESETS, type TaskItem, sortLinearIssues, - groupLinearIssues + groupSortedLinearIssues } from './mobile-tasks-legacy-foundation' export function useMobileTasksProviderViewProjection(model: PickerProjectionModel) { @@ -133,8 +133,8 @@ export function useMobileTasksProviderViewProjection(model: PickerProjectionMode [items, linearOrderBy] ) const linearIssueSections = useMemo( - () => groupLinearIssues(linearIssuesForView, linearGroupBy, linearOrderBy), - [linearGroupBy, linearIssuesForView, linearOrderBy] + () => groupSortedLinearIssues(linearIssuesForView, linearGroupBy), + [linearGroupBy, linearIssuesForView] ) // Why: FlatList treats data identity as meaningful; unrelated renders should // not rebuild the section/item wrapper array. @@ -155,9 +155,9 @@ export function useMobileTasksProviderViewProjection(model: PickerProjectionMode const linearBoardSections = useMemo( () => linearGroupBy === 'none' - ? groupLinearIssues(linearIssuesForView, 'status', linearOrderBy) + ? groupSortedLinearIssues(linearIssuesForView, 'status') : linearIssueSections, - [linearGroupBy, linearIssueSections, linearIssuesForView, linearOrderBy] + [linearGroupBy, linearIssueSections, linearIssuesForView] ) const githubModeLabel = githubMode === 'project' ? 'Projects' : githubKind === 'prs' ? 'PRs' : 'Issues'