You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/09/08 14:13:45 UTC
[I] tracking(perf): bound storage and protocol read/write amplification [maka]
Astro-Han opened a new issue, #5038:
URL: https://github.com/apache/maka/issues/5038
## Problem
Some Storage and Runtime Host operations process substantially more data than the requested result or the new change requires. Repeated page serialization, full-record output updates, full-store metadata reads, and historical Usage materialization can spend CPU, memory, and I/O on unchanged or unrelated data.
This tracker owns the remaining delivery and measurement work identified by [Discussion #4876](https://github.com/apache/maka/discussions/4876). It is scoped to read/write amplification in Storage and the Host protocol, rather than every performance issue in Maka.
## Scope and relationship to existing work
- **This tracker:** rows scanned/materialized/decoded, bytes encoded and written, and how those costs scale with the requested page, changed data, and stored history.
- **#4677:** user-visible Desktop and Host outcomes such as startup, Session opening, scrolling, and reading-position preservation. A storage improvement does not by itself satisfy those latency and interaction budgets.
- **#2913:** incremental Session data flow. Reuse its existing work where the same path is involved.
- **Discussion #4876:** the underlying rules, accepted exceptions, and unresolved policy questions. This issue does not turn every proposal in that discussion into an approved implementation requirement.
Use existing issues and PRs for their concrete work. Links here record dependencies and acceptance; they do not create a second implementation or a second owner.
## Current evidence
Source inspection baseline: [`87797378c`](https://github.com/apache/maka/commit/87797378cec89e2a31351f64656dc8d26a10e73d). The observations below are code-path evidence, not new production latency measurements.
### Repeated encoding and page assembly
Several page builders append one item, serialize the entire candidate page, and discard that encoding after checking its byte length. Examples:
- [Memory entries](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/runtime-host/src/server/memory-projection.ts#L153-L174).
- [Artifact pages](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/runtime-host/src/server/artifact-coordinator.ts#L502-L527).
- [Plan pages](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/runtime-host/src/server/plan-coordinator.ts#L300-L322).
- [Session catalog pages](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/runtime-host/src/server/session-catalog-coordinator.ts#L1442-L1474).
- [Runtime resource pages](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/runtime-host/src/server/runtime-resource-projection.ts#L61-L95).
For similarly sized items, cumulative serialization work grows quadratically with the number of items admitted to a page. Existing count/byte caps limit several of these paths; this is not a claim of unbounded whole-history work or a measured user-visible regression in every page builder.
Byte-limit helpers also retain private copies alongside `protocol/codec.ts`. Consolidation alone is not performance acceptance: measure actual repeated serialization along producer/consumer paths. The transport already accepts `EncodedProtocolMessage`; extend that seam where applicable rather than adding a parallel encoding path. Raw frame bytes, normalized values, and nested-object budgets are distinct and must not be substituted for one another.
### Shell output persistence
[ShellRunManager.queuePersist](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/runtime/src/shell-run-manager.ts#L1118-L1157) compares the output snapshot with the current record and calls `updateShellRun`. [Storage](https://github.com/apache/maka/blob/87797378cec89e2a31351f64656dc8d26a10e73d/packages/storage/src/shell-run-store.ts#L80-L100) reads the stored record and serializes the complete next record into `record_json`.
The full-record operation is confirmed. Its practical amplification remains to be measured against retained-output bounds, flush frequency, and SQLite write/WAL bytes before selecting a representation change.
### Existing delivery paths
| Work | Existing issue / PR | Remaining acceptance |
| --- | --- | --- |
| Artifact point reads and revisions | #4037 / #4874 | Verify bounded materialization when unrelated Sessions grow and when the target Session grows; preserve revision consistency and cross-process visibility. |
| Batch artifact cleanup during Session retirement | #4038 / #4984 | Verify batch work avoids repeated whole-store guard scans while preserving physical deletion safeguards and retryability. |
| Revision-consistent bounded Usage reads | #4058, #5019 / #5023; related UI work #4531 / #4539 | #5023 is currently a design-only draft. Resolve its Storage/query boundaries, then measure SQL scan/sort work as well as application decoding and transfer. UI pagination alone is insufficient. |
## Delivery checklist
- [ ] Deliver the first encoding/page-assembly change through existing protocol seams. Record the concrete covered paths, remove replaced implementations in the same change, and attach same-load before/after evidence. Keep this as one PR unless an actual dependency blocks part of the work.
- [ ] Measure Shell output persistence at representative retained-output sizes and flush rates. Record either a justified follow-up with its recovery contract or an explicit decision that the bounded cost is acceptable.
- [ ] Accept Artifact point-read growth behavior through #4037 / #4874.
- [ ] Accept batch retirement behavior through #4038 / #4984.
- [ ] Accept the Storage/protocol portion of Usage bounded reads through #4058 / #5023, including the All-range scenario in #5019. Keep the unresolved design decisions in that work rather than preselecting them here.
## Already changed; do not reopen from the old inventory
- Scheduled-task point operations no longer rewrite the full catalog: #4954 / #4955.
- Host Ready is followed by bounded background context-offload GC and maintenance: #4963. The old statement that GC is reached only through retirement/copy disposal is no longer current.
- Transcript query and navigation changes landed through #4879 and subsequent fixes. Old findings and counts in #4876 require current-source verification before becoming new work.
## Deferred questions
- Owner-lock/control-directory cleanup requires proof that removal preserves mutual exclusion; coordinate with #4712. Do not treat unlinking lock files as an ordinary cleanup optimization.
- Usage retention after Session deletion is a product-policy decision, not permission to delete historical records as part of a performance fix.
- Other tables without retirement paths are not automatically implementation tasks. Add work only when growth or an operational obligation is established.
- A repository-wide CI ratchet or mandatory exception ledger is not a prerequisite for the first fix and is not approved by this tracker.
## Acceptance
For each delivered path:
1. Name the real producer, authority, and downstream consumer. Measure through that path rather than only benchmarking a helper.
2. Compare the same workload on explicit before/after commits. Vary the relevant dimension: page size, target history, unrelated records, retained output, or retired Session count. Include a cold/warm distinction where relevant.
3. Report applicable rows scanned/decoded, serialization work, retained memory, write/WAL bytes, and elapsed time. For timing comparisons, use repeated runs and report median/p95; distinguish synthetic measurements from production observations.
4. Preserve UTF-8 byte budgets, completeness/cursor semantics, revision consistency, atomic durable facts, and retry/restart behavior relevant to the changed path. Retain focused behavioral tests that fail on the replaced behavior.
5. Do not accept moving a full scan from reads to every write, materializing everything behind a paginated API, or adding a second durable authority as a reduction in amplification.
Completion means each delivery item has a verified result or a documented, evidence-backed scope decision. It does not mean every read is constant-time, every historical row is deleted, or the separate user-experience budgets in #4677 have been met.
AI assistance: Codex helped inspect current source and draft this tracker under maintainer direction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [I] tracking(perf): bound storage and protocol read/write amplification [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on issue #5038:
URL: https://github.com/apache/maka/issues/5038#issuecomment-5592389868
## Shell output persistence baseline
I measured the current Shell persistence path before selecting a representation change.
### Scope and environment
- Commit: `9cb5cc93c03f8d07550e739164d5e33168892563`
- Apple M4 Pro, macOS arm64
- Node.js `v24.14.0`
- Store measurements exercised the real `SqliteShellRunStore.updateShellRun()` path with the operational `runtime.sqlite` schema, WAL mode, and `synchronous=FULL`.
- Each Store scenario performed 30 updates and was repeated in three fresh temporary workspaces. The table reports the middle result across the three rounds.
- The Runtime measurements exercised the real `ShellRunProcessManager -> ShellRunStore -> SQLite` path with real child processes and the default 1-second / 64-KiB flush policy.
- WAL auto-checkpointing was disabled during each measured interval so WAL growth remained observable.
### Isolated Store results
| Retained record / change per update | Update p50 | Update p95 | JSON serialization amplification | WAL amplification |
| --- | ---: | ---: | ---: | ---: |
| 64 KiB retained / 64 KiB changed | 0.32 ms | 0.58 ms | 1.0x | 1.1x |
| 1 MiB retained / 64 KiB changed | 3.90 ms | 4.86 ms | 16.0x | 12.5x |
| 1 MiB retained / 4 KiB changed | 1.74 ms | 3.88 ms | 256.1x | 26.9x |
| 2 MiB across stdout/stderr / 4 KiB changed | 3.77 ms | 6.70 ms | 512.1x | 28.1x |
The amplification ratios compare the sum of complete encoded `ShellRunRecord` sizes, or WAL growth, with the amount of newly changed output.
Metadata-only updates still process the complete output-bearing record:
| Retained record | Update p50 | Update p95 | WAL growth over 30 updates |
| --- | ---: | ---: | ---: |
| 1 MiB, metadata-only update | 1.41 ms | 4.25 ms | 1.25 MiB |
| 2 MiB, metadata-only update | 3.48 ms | 6.02 ms | 2.26 MiB |
### End-to-end Runtime path
| Workload | Durable output | Store updates after entering running state | Sum of resulting record sizes | WAL growth |
| --- | ---: | ---: | ---: | ---: |
| Burst 2 MiB stdout | ~1,020 KiB retained tail | 2 | 1.99 MiB | 2.03 MiB |
| 4 KiB every 250 ms for 3 seconds | 48 KiB | 4 | 0.13 MiB | 0.18 MiB |
| Fill ~1 MiB, then 4 KiB every 1.1 seconds | ~1,008 KiB retained tail | 6 | 5.94 MiB | 6.06 MiB |
For the saturated-tail workload, the Store update p50 was 4.38 ms and p95 was 6.90 ms. Approximately 16 KiB of output arrived after saturation, while the persistence path processed about 6 MiB of complete records and produced about 6 MiB of WAL traffic.
### Interpretation
The amplification is real but bounded by the existing retained-output limits and flush coalescing:
- A short burst is relatively well coalesced. The 2 MiB burst caused one main output/final-state write plus one later observation write.
- The expensive case is a long-running command whose retained tail is already near its limit and then changes slowly. A few KiB of new output repeatedly causes a roughly 1 MiB record to be read, parsed, compared, serialized, and updated.
- Small metadata changes such as `observedAt` also retain the full JSON processing cost because output and lifecycle metadata share one `record_json`.
- WAL amplification is lower than JSON serialization amplification because SQLite writes changed pages rather than an abstract full JSON byte stream, but the measured 4-KiB cases still produced approximately 27-28x WAL amplification.
This is a local synthetic Storage/Runtime measurement, not a production latency claim. Disabling auto-checkpointing makes cumulative WAL writes visible; the numbers should not be interpreted as steady-state WAL file size.
The results justify a follow-up, particularly around separating small lifecycle metadata updates from the large output payload and reducing saturated-tail rewrites. They do not yet demonstrate that a more complex append-log or checkpoint-plus-delta representation is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [I] tracking(perf): bound storage and protocol read/write amplification [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on issue #5038:
URL: https://github.com/apache/maka/issues/5038#issuecomment-5595746033
## Proposed Shell output persistence plan
The measurements support treating two costs separately:
1. Metadata-only mutations currently read, parse, compare, serialize, and update a record containing the full output payload.
2. Once the retained output tail is near its limit, small continuing output changes still cause repeated complete output checkpoints.
The first problem can be addressed without changing the durability contract. The second requires an explicit decision about the acceptable crash-loss window before changing flush behavior.
### Invariants
The implementation should preserve the following:
- `ShellRunStore` remains the sole ShellRun state-machine authority.
- Revisions remain monotonic and advance only for real durable changes.
- Final status and final output are committed atomically.
- Live Read/inspect continues to establish and persist a current snapshot cut before returning.
- Terminal outcomes remain immutable.
- Cross-process visibility and restart/orphan recovery behavior remain unchanged.
- PTY output remains a complete screen checkpoint unless separate evidence justifies a different representation.
## Phase 1: isolate metadata and output in the same row
This phase addresses metadata/output coupling only. It is not intended to eliminate the main saturated-tail output rewrites.
### Schema
Keep one row per ShellRun, but replace the combined `record_json` with two payload columns:
```sql
CREATE TABLE core_shell_runs (
session_id TEXT NOT NULL,
shell_run_id TEXT NOT NULL,
started_at INTEGER NOT NULL,
metadata_json TEXT NOT NULL,
output_json TEXT NOT NULL,
PRIMARY KEY (session_id, shell_run_id)
);
```
`metadata_json` contains the ShellRun identity, command, lifecycle state, timestamps, revision, and sandbox fields. `output_json` contains only `PipeShellOutput` or `PtyShellOutput`. `started_at` remains a scalar projection for Session ordering.
Using two columns in the existing row is preferable to adding a second table: SQLite transactions could keep two rows atomic, but a second table would add a cross-row existence and synchronization invariant without providing a benefit needed by this change.
### Interface and ownership
Keep the existing `ShellRunStore` interface unchanged:
```ts
createShellRun(record): Promise<ShellRunRecord>
updateShellRun(sessionId, shellRunId, patch): Promise<ShellRunRecord>
readShellRun(sessionId, shellRunId): Promise<ShellRunRecord>
listSessionShellRuns(sessionId): Promise<ShellRunRecord[]>
```
The SQLite adapter should own the physical split and reconstruction. Runtime should not learn about columns, encoding, migration, or storage-specific merge rules. `nextShellRunRecord()` remains the single implementation of revision, transition, no-op, and terminal-outcome rules.
An internal Storage codec should encode and decode the two persisted parts. Migration and normal reads/writes should use the same codec rather than defining a second interpretation of the durable format.
### Update behavior
Within the existing write transaction:
1. Read and reconstruct the current complete record.
2. Apply `nextShellRunRecord(current, patch)`.
3. Return unchanged on a no-op.
4. Always encode the changed metadata.
5. Update `output_json` only when the normalized output actually changed.
A metadata-only mutation should use an update equivalent to:
```sql
UPDATE core_shell_runs
SET started_at = ?, metadata_json = ?
WHERE session_id = ? AND shell_run_id = ?;
```
An output mutation should update both payload columns in one statement, preserving the current single-row atomicity.
Because `updateShellRun()` still returns a complete `ShellRunRecord`, Phase 1 should not claim that metadata-only updates avoid every output read/parse. It does guarantee that they avoid output serialization, output-column writes, and changes to the output's SQLite pages. Removing the remaining parse would require an interface/ownership change and should not be hidden inside this migration.
### Migration
- Bump `SQLITE_CORE_EXECUTION_SCHEMA_VERSION` from 9 to 10.
- Rebuild `core_shell_runs` using the target two-column shape.
- For every v9 row, parse and normalize the old `record_json`, then encode `metadata_json` and `output_json` using the production codec.
- Only after all rows are converted should the old table be removed and the replacement renamed.
- Recreate `core_shell_runs_session_order`.
- Update the operational schema registry and require `assertCurrentOperationalTargetSchema()` to accept the exact final shape.
- Perform the complete migration inside the existing operational-state migration transaction. A malformed old row or any migration failure must roll back to the unchanged v9 database.
The migration should also report representative existing-row count, elapsed time, peak temporary disk use, and DB/WAL growth.
### Phase 1 acceptance
- Pipes and PTY records round-trip through create/read/list/update.
- Metadata-only updates do not execute an `UPDATE OF output_json` and do not change the output payload.
- Output-only and combined mutations preserve revision semantics.
- No-op mutations do not advance revision.
- Output mode remains immutable.
- Terminal outcomes remain immutable and `observedAt` remains first-write-only.
- Malformed metadata/output and disagreement between row identity and JSON identity are rejected.
- Cross-process commits remain visible.
- v9 migration covers pipes, PTY, multiple Sessions, ordering, malformed-row rollback, reopen, and idempotent current-schema startup.
Performance evidence should compare explicit before/after commits for 1 MiB and 2 MiB metadata-only updates, output-changing updates, terminalization, and observation writes. Report encoded bytes, WAL growth, transaction median/p95, and whether `output_json` was written.
## Decision gate: define the durability contract
Before changing the automatic flush cadence, maintainers need to decide:
> If the Runtime Host terminates abruptly while a ShellRun is active, what is the maximum acceptable amount of output that may not yet be durable? Is the current approximately one-second interval an intended recovery contract, or only an implementation default?
Until that is answered, Phase 1 should retain the current defaults:
```ts
DEFAULT_SHELL_RUN_FLUSH_INTERVAL_MS = 1_000
DEFAULT_SHELL_RUN_FLUSH_BYTES = 64 * 1024
```
The existing live Read path already forces a current snapshot through `persistObservation(live)` before returning. Any later cadence change must preserve and strengthen tests around that behavior. Stop, finalization, foreground return, and normal shutdown must likewise retain their durable cuts.
## If a larger bounded crash-loss window is accepted
Use adaptive full-checkpoint coalescing rather than a new log format:
- Persist immediately when unpersisted output reaches the existing 64 KiB threshold.
- Otherwise persist when the oldest unpersisted output reaches an explicit maximum age, for example five seconds.
- Force persistence on live Read/inspect, stop, finalization, foreground return, normal shutdown, and PTY control operations that require a durable snapshot cut.
- Keep generation tracking so output arriving during an in-flight flush schedules the next flush.
- Commit final output, status, exit code, completion time, and revision atomically.
The documented recovery contract should state that an abrupt Host loss may recover only the last committed checkpoint, while normal observation and termination paths do not intentionally lose admitted output.
Tests should cover byte-triggered flushes, age-triggered flushes, Read cancelling/replacing a pending timer, output arriving during an in-flight write, final atomic settlement, persistence failure, and simulated restart from the last committed checkpoint.
## If the approximately one-second durability window must remain
Do not weaken recovery semantics by simply increasing the timer. First extend the measurement to establish whether the bounded cost is acceptable:
- At least 60 seconds after a 1 MiB tail has saturated.
- 4 KiB/s and 64 KiB/s output rates.
- stdout-only and stdout/stderr workloads.
- 1, 8, and 32 concurrent ShellRuns.
- Default WAL auto-checkpoint behavior.
- CPU, RSS, WAL/checkpoint work, transaction median/p95, and shared-database write-lock wait.
- Impact on unrelated operations sharing `runtime.sqlite`.
If that evidence still shows unacceptable interference, the next design should be pipes-only checkpoint plus bounded deltas. It would need an explicit recovery and compaction contract covering stdout/stderr ordering, retained-tail prefix removal, redaction across chunk boundaries, oversized-line safety, atomic revision publication, and interrupted compaction. PTY should remain on complete screen checkpoints because terminal control sequences can rewrite existing state and are not naturally append-only.
## Delivery sequence
1. Land same-row `metadata_json` / `output_json` isolation and the v9-to-v10 migration.
2. Obtain an explicit decision on the active-ShellRun crash-loss window.
3. Run the longer and concurrent measurements needed for an evidence-backed scope decision.
4. If a larger window is accepted, implement adaptive full-checkpoint coalescing.
5. Only if the one-second window must remain and measured interference is unacceptable, consider a pipes-only delta representation.
The guiding rule is to remove metadata/output write coupling first, then let the durability contract determine whether complete output checkpoints may be coalesced. The performance change should not silently broaden the recovery-loss window or introduce a chunk system before the simpler options and measured bounds are exhausted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [I] tracking(perf): bound storage and protocol read/write amplification [maka]
Posted by "ggbdpq (via GitHub)" <gi...@apache.org>.
ggbdpq commented on issue #5038:
URL: https://github.com/apache/maka/issues/5038#issuecomment-5872914867
**Status of delivery checklist item 5 - "Accept the Storage/protocol portion of Usage bounded reads through #4058 / #5023, including the All-range scenario in #5019": every anchor is closed, only the checkbox is waiting.**
Anchor chain, all verified closed:
| anchor | how it settled |
| --- | --- |
| #4058 (revision-consistent Usage reads) | implemented by #5387 (`c980b93a3`): single read transaction + revision fence + keyset cursor; the wire contract from the closed #5023 design (`screen_response_too_large` section/entry/message caps) landed there too, with coverage in `usage-screen.test.ts` |
| #5019 (All-range hang) | closed 09-18 - the original reporter re-verified on an existing 6,544-record profile: All range returns a bounded 50-row first page (the original bug was unbounded renderer materialization: 4.4GB footprint, pegged core, unresponsive Cmd+Q) |
| #5515 (SQL-layer successor: page queries still scanning the residual time range) | **fixed by #5514**: per-source indexed seek taking <=51 candidates into a global merge, no-filter counts reusing full statistics, zero-count short-circuit |
| #5517 (filter changes recompute range statistics) | merged 09-25: statistics projection cached by revision+range, repeated aggregation 5 -> 0 |
Against this tracker's acceptance wording for the item ("measure SQL scan/sort work as well as application decoding and transfer"):
- **SQL scan/sort: measured** - #5514 recorded EXPLAIN QUERY PLAN (index seeks on (ts, storage_key)), outer temp B-tree input bounded at <=153 candidates, and function-evaluation counts (50k-record paging: 99,900 -> 51 lowercase evaluations), plus deterministic work regressions that fail on the baseline.
- **Application decode: measured** (via the real `readUsageScreen` facade including row decode) - 50k-record paging median 53.5ms -> 0.8ms, independent of history depth.
- **Transfer: bounded by construction, not separately measured** - the #5387 wire caps and the `screen_response_too_large` rejection path are tested, so transfer cannot grow unboundedly with history; raw wire bytes were however never recorded as a number (#5514 explicitly excluded wire serialization). If the literal number is wanted, extending `scripts/perf/usage-pages.mjs` to serialize `readUsageScreen` results through `protocol/codec.ts` and report bytes per page/screen is a small addition - happy to do it.
Unless the maintainers want that last number, this item looks ready to tick. Source inspection on current main confirms the shape: per-source `ORDER BY ts DESC ... LIMIT 51` subqueries merged globally, statistics reuse, cursor trimmed for two-column index seeks; the remaining range-level scans (full statistics, exact filtered counts, sparse substring search) are the explicitly exempted ones.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [I] tracking(perf): bound storage and protocol read/write amplification [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on issue #5038:
URL: https://github.com/apache/maka/issues/5038#issuecomment-5592285584
Thanks @Astro-Han for pulling these performance points together into a well-scoped tracker — the clear boundaries around scope, the acceptance criteria, and the "already changed; do not reopen" section make this a genuinely useful read.
I'd like to pick up the **second checklist item: Shell output persistence measurement**. I'll confirm the paths against current `main` first.
As the tracker frames it, this is a *measure-first* task, not a predetermined representation change. My plan follows the Acceptance criteria:
**1. Path (producer → authority → consumer).**
- Producer: `ShellRunManager.queuePersist` — takes a full output snapshot, deep-compares the candidate record, and on any diff calls `updateShellRun` with the complete `output`.
- Authority: `SqliteShellRunStore.updateShellRun` — reads the current record, merges, and `JSON.stringify`s the whole `next` record into the `record_json` column (one atomic `UPDATE`).
- Consumer: `readShellRun` / `listSessionShellRuns` on restart, plus the live shell-run views. Recovery reconstructs the retained tail and truncation flags from that single blob.
**2. What I expect to find, and want to quantify.** The retained output is already bounded (`BASH_MAX_RETAINED_CHARS`, ~1MB/stream), so *per-flush* cost is bounded. The amplification is in the **flush count**: with a ~1s / 64KB flush cadence, a long-running command that produces far more than the retained cap rewrites the full (capped) record on every flush. So cumulative write/WAL work scales roughly as `(total_produced_output / flush_bytes) × retained_record_size`, even though only a tail is ever kept. I want to measure whether that is material at representative sizes rather than assume it.
**3. Measurement.** Same-workload before/after on explicit commits, varying:
- retained-output size (up to the cap) and total produced output (drives flush count),
- flush cadence (interval vs byte trigger).
Reporting per-flush `JSON.stringify` bytes, `isDeepStrictEqual` cost, SQLite/WAL bytes written, retained memory, and repeated-run median/p95 elapsed, with a cold/warm distinction.
**4. Deliverable.** Per the checklist, one of: (a) a justified follow-up with an explicit **recovery contract** — preserving the atomic durable fact, exact reconstruction of the retained tail and truncation flags, and retry/restart behavior — or (b) an explicit, evidence-backed decision that the bounded cost is acceptable. I will **not** move a full scan onto every write, add a second durable authority, or weaken atomicity/recovery as a "reduction."
I'll keep performance claims limited to the measured paths. Happy to align if a maintainer prefers I fold this into existing shell-run work instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [I] tracking(perf): bound storage and protocol read/write amplification [maka]
Posted by "chinawch007 (via GitHub)" <gi...@apache.org>.
chinawch007 commented on issue #5038:
URL: https://github.com/apache/maka/issues/5038#issuecomment-5588150087
I’d like to take on the **first delivery checklist item: encoding and page-assembly optimization**.
My initial scope is the five page-building paths identified here: **Memory entries, Artifact pages, Plan pages, Session catalog pages, and Runtime resource pages**. I’ll verify these against current `main` and check for overlapping work before making changes.
My proposed approach is:
1. **Trace the existing paths and establish a baseline.**
Follow page construction through protocol validation, outbound encoding, transport, and the downstream consumer. Identify repeated serialization and array copying, and measure the work performed by the actual request paths rather than only benchmarking a helper.
2. **Replace repeated candidate-page serialization with incremental byte accounting.**
Evaluate a shared implementation that measures normalized items once and accounts for the page envelope, separators, and changing cursor fields. Reuse encoding results or byte measurements where they describe the same representation, through the existing protocol interfaces and `EncodedProtocolMessage` seam.
3. **Apply the change consistently and remove superseded implementations.**
Cover the identified page builders in one PR unless a concrete dependency blocks part of the work. The PR will explicitly document the paths covered and remove the implementations it replaces.
4. **Verify behavior and attach reproducible performance evidence.**
Add focused tests for UTF-8 and JSON escaping, exact byte boundaries, oversized items, empty/final pages, and cursor transitions. Check pagination completeness and revision consistency. Compare explicit before/after commits under the same workloads, varying page size and item size, and report serialization work, memory, and repeated-run median/p95 timings, with cold/warm distinctions where relevant.
The key constraints I’ll preserve are the existing count/byte limits and validation behavior. **Raw frame bytes, normalized result values, and nested-object budgets will remain distinct**; reusing an encoding measurement must not bypass a different validation boundary. Performance claims will be limited to the measured paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org