Skip to content

perf(host): export only the requested plugin package - #6661

Merged
M4n5ter merged 2 commits into
rust-previewfrom
perf/export-one-package
Nov 6, 2026
Merged

M4n5ter merged 2 commits into
rust-previewfrom
perf/export-one-package

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

The workbench imports each UI plugin by exporting its package through management.exportPackage. The Host served every export by loading the whole committed source. That means reading every installed package from SQLite, recomputing every package digest and validating the plan again, while holding the manager's lock. A default installation has 25 packages (about 9 MB). Each plugin import therefore paid for all of them, and the five imports at desktop startup ran one after another behind that lock.

exportPackage now reads only the requested package of that revision, through the new RootStore::load_application_package_at. The other packages of the revision are no longer read. Behaviour is otherwise the same:

  • a foreign source is still rejected;
  • a package the revision does not contain is still reported as not installed;
  • the exported bytes are still checked against their digest.

Measured with a temporary probe (not part of this PR) in the desktop real-host fixture, from Desktop::open until the composer's workspace picker is shown. The plugin load chain is the time from the UI catalogue arriving until the last startup plugin is active.

before after
release: open to workspace picker (median of 3) 980 ms 813 ms
release: plugin load chain 561 ms 256 ms
release: one package export 78–180 ms 1–90 ms
debug: open to workspace picker (2 runs) 6.5 s 5.0 s
debug: plugin load chain 3.3 s 1.7 s

Debug builds hash without optimisation, so the real-host tests gain about 1.5 s every time they open a desktop.

AI use

  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code measured the startup timeline, found the whole-source load in exportPackage, and wrote the change, the tests and this description. The commit carries Generated-by: Claude Code.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed the complete four-file change, the existing application commit/read path and SQL schema, read_package, and the manager's previous source_at checks.

The change has a concrete cost-saving mechanism: exportPackage previously loaded and hashed every package and rebuilt the revision's plan for each requested export. It now checks membership of the requested package in the exact deployment revision and reads only that package. The manager retains both state-domain and deployment checks; storage validates the source and package references, RootStore still requires an open owner, and read_package still validates paths, BLOB payloads and the reconstructed package digest. Exporting retained committed revisions continues to work without granting membership from another revision or deployment.

I found no confirmed P0–P2 issue in this change. I executed the exact new membership SQL against the real initial schema in a synthetic SQLite database: original/current revision, absent revision, another package and another deployment cases passed. Separately, I read the original hosted test logs from CI 42164401964: the expanded immutable-revision test passed in job 127726921911, and the expanded corruption test passed in job 127726921874 (their complete shards passed 188/188 and 187/187). Their actual checkout is 17e7203962e9c8ff10c76feeea1880f197e6a8cc, the merge of this head with rust-preview@72162299; my merge-tree produces the same tree.

The overall CI run failed: native7 repeats the #6655 cancelled-instruction-ack fixture problem, and CLI1 times out waiting for the model/source-search UI at keyboard.rs:312. I read both logs; I have not attributed the UI wait to this export change, so it is not an additional P2 finding here. The merge is clean. No protocol or database migration changes.

I did not run the complete Rust suite locally, a native desktop startup, a real Host package stream, or an independent performance benchmark. The release/debug timing improvements in the PR description are the author's measurements, not mine.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

management.exportPackage loaded the whole committed source for every
export: all installed packages read from SQLite, every digest recomputed
and the plan validated again, under the manager's lock. The workbench
exports one package per UI plugin while it starts, so each import paid
for every package of the installation (25 packages, about 9 MB by
default).

Read only the requested package of the revision instead. A foreign
source is still rejected, a package outside the revision is still
reported as not installed, and the exported bytes are still checked
against their digest.

In the release desktop fixture the startup plugin load chain drops from
about 560 ms to 260 ms, and the workspace picker appears about 170 ms
sooner.

Generated-by: Claude Code
@M4n5ter
M4n5ter force-pushed the perf/export-one-package branch from 0848b6c to 776b1b7 Compare November 6, 2026 09:37
@M4n5ter
M4n5ter merged commit 0111582 into rust-preview Nov 6, 2026
@M4n5ter
M4n5ter deleted the perf/export-one-package branch November 6, 2026 09:38

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I completed the follow-up review of the final eight-file change at 776b1b7, rather than extending the earlier four-file review to this larger scope without checking it.

The final version also moves package reconstruction/digest validation and full-source plan validation off the SQLite worker, reads package payloads in one query, applies the exact-package loader to readPackage and package-settings queries, reserves export resources before loading, and caches the immutable deployment-manager contract. I checked that manager domain/deployment validation, exact revision membership, path/BLOB validation, reconstructed digest checks, and RootStore's open-owner check remain. The transaction captures the immutable package bytes before the blocking worker validates them. The new sibling-corruption regression covers why a requested intact package no longer depends on validating an unrelated package.

I found no confirmed P0–P2 issue in these changes. I reran the exact production membership and payload SQL against this head's real initial schema in a synthetic SQLite database: six revision/deployment/package membership cases passed, the requested bytes remained independent of a corrupt sibling, and non-BLOB values were exposed as NULL for rejection. This checks SQL selection and type handling; it is not a Rust SQLx, digest-validation, cancellation or performance test. I read the complete final diff and compared the eight affected paths against the previously reviewed 0848 version, separately from incoming base changes. The actual merge of this head into 95df8f6 produces the exact tree merged as 01115825.

The final-head CI and merged-branch CI are still queued/running, so the new Rust tests have not been independently accepted here. The earlier hosted immutable-revision/corruption tests belonged to 0848 and do not establish this final head's result. I did not run the complete Rust suite, a real Host package export, or an independent desktop benchmark. No protocol or database schema changes. No approval or merge performed.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants