[Bug] The export dialog runs six full deck scans on the UI thread before it paints #613
Labels
No labels
accepted
bug
declined
docs
duplicate
enhancement
good first issue
in-progress
needs-info
privacy
security
triage
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
LibreKAT/Ocideck#613
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found in the pre-publication performance review.
Evidence:
lib/widgets/app_shell_main_layout.dart:769—bundleForperforms three unmemoised full deck scans per call:PrivacyProjection.forAudience(privacy_projection.dart:170),RedactionManifestService.build→redactedValues(redaction_manifest_service.dart:140), andsummarisePrivacyForExport(source, PrivacyScanner(…).scan(source))(app_shell_main_layout.dart:815), plusgenerateDeckand_expandForExport.It is called at
:821(forhasPrivacyFindings) and again atlib/widgets/dialogs/export_dialog.dart:166with the same arguments — both synchronously, before the dialog first frame. Cost per scan, by the repository own measurement inlib/services/privacy/privacy_scan_memo.dart:14: ~1.07 ms per slide, so ~208 ms at 200 slides, and that is the unexpanded count. Every profile change in the dialog (export_dialog.dart:172) costs another three.Why this matters now: six full scans plus two manifest builds on the UI thread is roughly a second and a half of frozen window at 200 slides, with no spinner, immediately after clicking Export. The rasterising path afterwards is exemplary — progress, phases and cancellation are fully wired (
slide_rasterizer.dart:97-102,export_dialog.dart:349-367) — so the only place the export feels like it has hung is the place that is not doing anything yet.Proposal: derive
hasPrivacyFindingsfrom the already memoisedprivacyRawScanProviderrather than from a full bundle, and letredactedValuesandsummarisereuse the projection scan within a singlebundleForinstead of repeating it. One scan per bundle, one bundle for the dialog.Opgepakt. Tak:
perf/exportdialoog-scans-613. Reikwijdte:app_shell_main_layout.dart(bundleFor+hasPrivacyFindings),privacy_projection.dartenredaction_manifest_service.dart(een al gemaakte scan mogen meekrijgen), plus tests.Opgelost in
b5ea59c2+35830c70+ de dekkingstest (PR #680), op main.Beide voorstellen uit je issue zitten erin:
hasPrivacyFindingskomt uitprivacyExportSummaryProviderin plaats van uit een hele bundel, enforAudience/RedactionManifestServicenemen desgewenst een al gemaakte scan mee. Laten ze hem weg, dan scannen ze zelf zoals voorheen — elke andere aanroeper blijft ongemoeid. Zes scans werden er één, plus één per profiel- of diepgangkeuze.De test bewaakt niet de winst maar de voorwaarde. Een scan tellen kan niet van buitenaf; of de meegegeven scan werkelijk gebruikt wordt wél. Een lege scan meegeven moet niets redigeren en een leeg manifest opleveren — doet de code dat niet, dan scande ze alsnog zelf en is de herhaling er gewoon nog, hoe groen de rest ook staat.
Twee dingen die erbij hoorden. De wijziging duwde
_MainLayoutStateover zijn klasseplafond, dus is de bundelfabriek verhuisd naarlib/services/export_bundle.dart— het is exportlogica, geen schermlogica, en hij hoort naastExportBundle, in het bestand waarvan de kop de projectiegrens al beschrijft die deze functie bewaakt. Daarmee viel meteen de dekkingsvloer: dat bestand stond op 6,7%, omdat de fabriek tot nu toe alleen via een widgettest bereikbaar was en daar nooit doorlopen werd. Nu is hij gewoon een functie met invoer en uitvoer, en heeft hij zeven tests. Dat is achteraf de grootste opbrengst van deze wijziging.Wat er níét in zit: het rasteren zelf blijft even duur; daar zat het probleem niet — dat pad had al voortgang, fasen en annuleren.