Skip to content

add persistence controls for lower SSD writes - #555

Open
Skater1808 wants to merge 7 commits into
JustVugg:devfrom
Skater1808:feat/no-persist-ssd
Open

add persistence controls for lower SSD writes#555
Skater1808 wants to merge 7 commits into
JustVugg:devfrom
Skater1808:feat/no-persist-ssd

Conversation

@Skater1808

Copy link
Copy Markdown

Summary

Describe the problem and the smallest change that solves it.

Validation

  • make -C c check
  • CUDA changes were tested with make -C c cuda-test (if applicable)
  • Performance claims include hardware, commands, and repeatable measurements

Compatibility

  • The default CPU build remains dependency-free
  • No model files, generated binaries, or benchmark artifacts are included

@JustVugg

Copy link
Copy Markdown
Owner

Thanks for this — persistence controls for lower SSD writes is a useful feature. One process note before review: this PR targets main, but all contributions land on dev first (main is release-only and protected). Could you retarget the base branch to dev? On GitHub: Edit (top-right of the PR) → change base from main to dev. Once it's on dev I'll review it properly — the persistence/write-reduction angle is worth having, especially for the disk-streaming hosts.

@Skater1808
Skater1808 changed the base branch from main to dev July 23, 2026 18:29
@Skater1808

Skater1808 commented Jul 23, 2026

Copy link
Copy Markdown
Author

Thanks, I've already changed it. If you have any further questions, I'm here.

@JustVugg JustVugg added the enhancement New feature or request label Jul 28, 2026
@JustVugg

JustVugg commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Rebase request — and an apology for how many of these you have had.

dev moved a lot in the last day: 19 PRs landed, including the Vulkan backend (#418), the Kimi K3 GPU tier (#705), Metal grouped-int4 (#457), the shared routing telemetry (#716/#719), and several CUDA and launcher fixes. This PR now conflicts.

Before asking, three things changed on our side so that this is the last one of these you should need for a while.

1. The main source of these conflicts is closed. c/Makefile had a single hand-written TEST_BINS line listing every test gate. Every PR that added a test appended to that same line, so any two such PRs conflicted by construction, even when they touched entirely unrelated code — c/Makefile appeared in 26 of 40 open PRs. #386 hit it twice while being rebased, and said so, which is what sent us looking.

Gates are now derived from the build rules (#733). Adding a test means adding your .c and its own rule, which land in different places in the file. There is no shared list left to conflict on.

2. We resolved what we could ourselves instead of asking. Eight PRs were unblocked by a maintainer pushing the merge to the contributor's branch rather than requesting a rebase — including three where both sides carried a real change and had to be merged rather than picked. No commits were rewritten. Yours is here because its conflict is in engine code (c/colibri.c or a backend), where guessing your intent would be worse than asking.

3. Merge order is oldest-clean-first from now on. A PR that is green and unconflicted merges ahead of anything opened after it. The reason some of you rebased many times is that newer, smaller PRs kept jumping the queue and resetting you — a starvation loop we built, not bad luck on your side.

What we need: one rebase onto current dev. If your only conflict was the TEST_BINS line, just drop your entry — your test is picked up by its own rule now.

If you would rather not, say so and we will close it with thanks and the branch stays yours to reopen. No pressure either way, and no hard feelings — several of these have been open a while through no fault of the author.

GitHub Copilot added 4 commits August 1, 2026 21:34
- Introduced `upstream_telemetry.h` to handle telemetry data related to model layers, expert usage, and hardware statistics.
- Implemented functions for calculating memory usage, probing hardware specifications, and emitting statistics for GPU and CPU.
- Added support for tracking expert hits and usage statistics for better performance monitoring.
- Included conditional compilation for different operating systems (Windows, macOS, Linux) to gather hardware information.
@Skater1808
Skater1808 force-pushed the feat/no-persist-ssd branch from 595ef1b to d35067c Compare August 1, 2026 19:39
@Skater1808

Copy link
Copy Markdown
Author

Rebased onto current dev, resolved the conflicts in the persistence-related launcher/telemetry changes, and pushed the updated branch so the PR is fresh again.

@JustVugg

JustVugg commented Aug 2, 2026

Copy link
Copy Markdown
Owner

This sat with CI never having run at all. GitHub held the workflow in action_required — a first contribution from a fork — and that state is not visible anywhere on the pull request page, so neither of us could see it. Approved now, eleven days late. Sorry.

The four failures are staleness, not your change. The Python one:

ERROR: test_absent_cap_stays_absent
AttributeError: 'types.SimpleNamespace' object has no attribute 'ram'

I checked before writing this: dev is green on that test today, and the a.ram read in cmd_chat's banner dates from the project's first commit. So the namespace your test builds is missing a field the CLI has always read — the branch is from 23 July and dev has moved. A rebase should take all four green.

One other thing while you are in there: the PR carries diff.txt, diff_telemetry.txt, head_coli, head_telemetry.h, upstream_coli and upstream_telemetry.h. Those look like working files from producing the diff rather than anything you meant to ship.

On the change itself — persistence controls for lower SSD writes is a real need and nobody has addressed it. This engine writes .coli_usage after every turn and .coli_kv alongside it, on a machine whose whole design assumption is that the model lives on an SSD. Someone running it continuously is paying write endurance for telemetry they may not want. That is worth having and worth having configurable.

Rebase on current dev and I will review it properly.

@JustVugg JustVugg added the needs-rebase Confligge, serve rebase dell'autore label Aug 2, 2026
@Skater1808

Copy link
Copy Markdown
Author

Rebased on dev, cleaned up the leftover working files, and fixed the stream handling for minimal test namespaces so everything runs green now. Ready for review whenever you have time!

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

Labels

enhancement New feature or request needs-rebase Confligge, serve rebase dell'autore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants