Do a performance and memory consumption optimization pass #91

Closed
opened 2026-08-16 00:21:03 +00:00 by Grandiras · 2 comments
Owner
No description provided.
Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/253); the reply will link the full session transcript. <!-- claude-run:comment --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/issue-91/run-253.md) — updating every ~5 minutes while the task runs.
Collaborator

Pushed to claude/issue-91.

What I found

There was no toolchain in the container, so I installed .NET 10 and PostgreSQL, seeded a board the size the group might actually run (4 columns, 120 cards, 2 tags + 2 assignees + 5 comments a card), and measured before touching anything. One thing dominated everything else.

CardService.GetColumnsAsync hung three collections off Card in one query, and collection includes multiply. That query came back 2,400 rows wide, each repeating that card's encrypted title and description — 2.85 MiB of ciphertext for 120 cards' worth of text. The third collection bought a single number: a card tile shows how many comments there are and never a word of them, so 600 comment bodies were fetched and AES-decrypted to be thrown away. And this runs on every board load and again in every connected circuit on every BoardNotifier event — one member dragging a card paid for it once per person looking.

Changes

Count the comments, don't load them. Card.CommentCount comes from a GroupBy returning two integers a card; Card.Comments is documented as not populated by the board read. Tags and assignees deliberately stay in the one query — four rows a card is cheaper than the two extra round trips AsSplitQuery spends avoiding it (~18 ms against ~25 ms). It was the third collection, not the join.

AesGcmDataEncryptor's string paths no longer route through EncryptBytes/DecryptBytes, which forced an array per call that existed only to be copied out of again. The envelope is unchanged — the new tests carry envelopes written by the old implementation to prove it. Buffers are pooled, and the ones holding plaintext are returned cleared.

BoardService.ResolveAsync asks "the board, if it is theirs" in one query instead of "the board" then "may they see it", which always cost two round trips on two connections.

CardPreview moved out of CardView.razor (untestable there, as BoardFilter was) and stops flattening 16,000 characters to render 200.

before after
board load, 120 cards ~68 ms / 8.0 MiB ~18 ms / 2.1 MiB
calendar read 0.98 MiB 0.74 MiB
encrypt / decrypt a 780-char value 6.9 KB / 5.4 KB 2.2 KB / 1.6 KB
card preview, one board render 0.60 MiB 0.34 MiB

Allocation figures are deterministic; the timings varied run to run on this shared box (the board load measured 12–24 ms after, 64–85 ms before), so treat those as approximate.

I measured and rejected one option: holding a single AesGcm instead of building one per call is ~25% faster per call, but AesGcm isn't documented thread-safe and this is a singleton every circuit shares — so it needs thread-affine caching of a native handle for the life of the process, to buy ~3% of a board load. Not worth it in that class.

Verification

  • 238 unit tests pass (205 before, 33 new — including the first coverage AesGcmDataEncryptor has ever had, with hard-coded legacy envelopes so a change to the bytes fails there rather than in production).
  • The preview rewrite is byte-identical, verified against the old implementation over 300,000 random descriptions at four cut lengths. That fuzz earned its keep: it caught two real differences in my first attempt (a line of pure emphasis must still leave its separator, and marker removal cascades — _**_ loses ** and then vanishes as __). Both are now pinned by name in CardPreviewTests.
  • Ran the actual app against Postgres and drove it with Chromium. Comment badges read 3, 7, 5, 2, none, 12 — matching the seed exactly; adding a comment took a badge 3 → 4 with no reload; all three outcomes of the rewritten resolution are right (open board and restricted-board-as-admin render, restricted-as-member → denied, nonexistent → missing); the calendar renders.
  • dotnet ef migrations has-pending-model-changes → no changes. CommentCount is [NotMapped], so there's no migration.

The board in the running app: comment badges reading 3, 7, 5, 2 and 12, with the zero-comment card showing none — the counts now come from a counting query instead of loading every comment

What I could not run: the Playwright suite needs podman/docker, and this CI image has neither. I extended board.spec.js's comment test to assert the badge (the one user-visible thing the query change could break, and nothing covered it) and made it create its own card — it mutates one, so by the repo's own rule it always should have. I then ran that test's exact flow by hand against the live app and it passes; it still deserves a real run in the compose stack.

Also added: a 2.4.1 changelog entry in both locales (a board that opens noticeably faster is member-visible, and "dragging is smoother" set that precedent in 2.4.0), and notes in CLAUDE.md recording the two new invariants — never a third collection include on Card, and the encryptor's allocation rules. One trap worth flagging that's now written down there: EF caches its compiled model, value converters and the encryptor they close over included, process-wide and keyed on the options rather than the DI container — so my first attempt at an in-process A/B of two encryptors silently measured the same one twice.


⏱ 44m 31s · 126 turns · tokens: 250.6k in / 100.2k out (+21360.9k cache-read)
📜 Full session transcript
→ Opened PR: #92

Pushed to **`claude/issue-91`**. ## What I found There was no toolchain in the container, so I installed .NET 10 and PostgreSQL, seeded a board the size the group might actually run (4 columns, 120 cards, 2 tags + 2 assignees + 5 comments a card), and measured before touching anything. One thing dominated everything else. `CardService.GetColumnsAsync` hung **three collections off `Card` in one query**, and collection includes multiply. That query came back **2,400 rows wide**, each repeating that card's encrypted title and description — **2.85 MiB of ciphertext for 120 cards' worth of text**. The third collection bought a single number: a card tile shows *how many* comments there are and never a word of them, so 600 comment bodies were fetched and AES-decrypted to be thrown away. And this runs on every board load *and again in every connected circuit on every `BoardNotifier` event* — one member dragging a card paid for it once per person looking. ## Changes **Count the comments, don't load them.** `Card.CommentCount` comes from a `GroupBy` returning two integers a card; `Card.Comments` is documented as not populated by the board read. Tags and assignees deliberately stay in the one query — four rows a card is cheaper than the two extra round trips `AsSplitQuery` spends avoiding it (~18 ms against ~25 ms). It was the third collection, not the join. **`AesGcmDataEncryptor`'s string paths no longer route through `EncryptBytes`/`DecryptBytes`**, which forced an array per call that existed only to be copied out of again. The envelope is unchanged — the new tests carry envelopes written by the *old* implementation to prove it. Buffers are pooled, and the ones holding plaintext are returned cleared. **`BoardService.ResolveAsync` asks "the board, if it is theirs" in one query** instead of "the board" then "may they see it", which always cost two round trips on two connections. **`CardPreview` moved out of `CardView.razor`** (untestable there, as `BoardFilter` was) and stops flattening 16,000 characters to render 200. | | before | after | |---|---|---| | board load, 120 cards | ~68 ms / **8.0 MiB** | ~18 ms / **2.1 MiB** | | calendar read | 0.98 MiB | 0.74 MiB | | encrypt / decrypt a 780-char value | 6.9 KB / 5.4 KB | 2.2 KB / 1.6 KB | | card preview, one board render | 0.60 MiB | 0.34 MiB | Allocation figures are deterministic; the timings varied run to run on this shared box (the board load measured 12–24 ms after, 64–85 ms before), so treat those as approximate. I measured and **rejected** one option: holding a single `AesGcm` instead of building one per call is ~25% faster per call, but `AesGcm` isn't documented thread-safe and this is a singleton every circuit shares — so it needs thread-affine caching of a native handle for the life of the process, to buy ~3% of a board load. Not worth it in that class. ## Verification - **238 unit tests pass** (205 before, 33 new — including the first coverage `AesGcmDataEncryptor` has ever had, with hard-coded legacy envelopes so a change to the bytes fails there rather than in production). - **The preview rewrite is byte-identical**, verified against the old implementation over 300,000 random descriptions at four cut lengths. That fuzz earned its keep: it caught two real differences in my first attempt (a line of pure emphasis must still leave its separator, and marker removal cascades — `_**_` loses `**` and then vanishes as `__`). Both are now pinned by name in `CardPreviewTests`. - **Ran the actual app** against Postgres and drove it with Chromium. Comment badges read 3, 7, 5, 2, none, 12 — matching the seed exactly; adding a comment took a badge 3 → 4 with no reload; all three outcomes of the rewritten resolution are right (open board and restricted-board-as-admin render, restricted-as-member → `denied`, nonexistent → `missing`); the calendar renders. - `dotnet ef migrations has-pending-model-changes` → no changes. `CommentCount` is `[NotMapped]`, so there's no migration. ![The board in the running app: comment badges reading 3, 7, 5, 2 and 12, with the zero-comment card showing none — the counts now come from a counting query instead of loading every comment](https://git.grandiras.net/attachments/63f55ace-6ba2-43fa-9133-e157cb9a3092) **What I could not run:** the Playwright suite needs `podman`/`docker`, and this CI image has neither. I extended `board.spec.js`'s comment test to assert the badge (the one user-visible thing the query change could break, and nothing covered it) and made it create its own card — it mutates one, so by the repo's own rule it always should have. I then ran that test's exact flow by hand against the live app and it passes; it still deserves a real run in the compose stack. Also added: a `2.4.1` changelog entry in both locales (a board that opens noticeably faster is member-visible, and "dragging is smoother" set that precedent in 2.4.0), and notes in `CLAUDE.md` recording the two new invariants — never a third collection include on `Card`, and the encryptor's allocation rules. One trap worth flagging that's now written down there: **EF caches its compiled model, value converters and the encryptor they close over included, process-wide and keyed on the options rather than the DI container** — so my first attempt at an in-process A/B of two encryptors silently measured the same one twice. --- ⏱ 44m 31s · 126 turns · tokens: 250.6k in / 100.2k out (+21360.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/issue-91/run-253.md) → Opened PR: https://git.grandiras.net/Grandiras/Ponente/pulls/92
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente#91
No description provided.