Skip to content

Commit 2b267b8

Browse files
g41797g41797
committed
Update design plan
1 parent ad7ba99 commit 2b267b8

File tree

2 files changed

+385
-1
lines changed

2 files changed

+385
-1
lines changed

‎.gitignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ ols.json
2424
mailbox_init.md
2525
design/STATUS.md
2626
design/heap-pool-plan.md
27-
last_plan.md
27+
# last_plan.md
2828
welcomed_tests.md
2929

3030
# Documentation tools

‎last_plan.md‎

Lines changed: 384 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,384 @@
1+
# Plan: Loop_Mailbox Enhancement — Multi-Package Library
2+
3+
## Context
4+
5+
`Loop_Mailbox` uses a mutex-protected intrusive list.
6+
This plan transforms it into a lock-free MPSC-based design and restructures
7+
the repository into a proper multi-package library with clean separation.
8+
9+
---
10+
11+
## Process
12+
13+
Implementation is iterative: one stage at a time.
14+
After each stage:
15+
- Full regression (build all + test all)
16+
- Rethink — update plan if needed
17+
- Docs, README sync, comments, AI-ish check
18+
- Update `design/STATUS.md` checkpoints
19+
- Update `last_plan.md` (full plan, not a diff)
20+
21+
---
22+
23+
## Key Decisions
24+
25+
- `Mailbox($T)` — copyable; stays in `mbox/`; no mpsc/wakeup dependency
26+
- `Loop_Mailbox($T)` — NOT copyable after init; moves to own `loop_mbox/` package
27+
- `loop_mbox/` is the "abstract base": generic queue + generic wakeup, no nbio
28+
- `nbio_mbox/` is the nbio concrete implementation of `loop_mbox/`
29+
- Extra import path from split is developer-side concern only, not client concern
30+
- Pool stays in this repo
31+
- **Unit tests**: white-box, inside the package folder, same package name
32+
- **Functional tests**: black-box public API, separate `*_tests/` folder (existing pattern)
33+
- **Examples in small packages** (mpsc/, wakeup/): `@(private) _example_*` procs in `*_test.odin`,
34+
called from thin `@(test)` wrappers. One file holds unit tests + examples. Safe — `@(test)` procs
35+
are excluded from `odin build` production output.
36+
- **examples/ folder**: stays as cross-package integration showcase (negotiation, stress, etc.)
37+
after Stage 6. Not split per-package.
38+
- **Per-package README.md**: deferred to Stage 8. Note for Stage 8: user wants per-package READMEs
39+
with snippets ("willings") — do not forget.
40+
- WakeUper is caller-initialized, passed to `init_loop_mailbox`, `loop_mbox` owns copy
41+
- Caller frees Loop_Mailbox struct memory; `close_loop` frees internal resources only
42+
43+
---
44+
45+
## Final package structure
46+
47+
```
48+
odin-mbox/
49+
mbox/ (Mailbox — blocking, no mpsc/wakeup deps)
50+
mbox.odin
51+
doc.odin
52+
mbox_test.odin (unit tests, package mbox)
53+
loop_mbox/ (Loop_Mailbox — generic, non-blocking)
54+
loop_mbox.odin
55+
doc.odin
56+
loop_mbox_test.odin (unit tests, package loop_mbox)
57+
nbio_mbox/ (nbio concrete impl of loop_mbox)
58+
nbio_mbox.odin
59+
doc.odin
60+
nbio_mbox_test.odin (unit tests, package nbio_mbox)
61+
mpsc/ (Vyukov lock-free queue)
62+
queue.odin
63+
doc.odin
64+
queue_test.odin (unit tests + @private examples + @test wrappers, package mpsc)
65+
wakeup/ (WakeUper interface + semaphore implementation)
66+
wakeup.odin
67+
doc.odin
68+
wakeup_test.odin (unit tests + @private examples + @test wrappers, package wakeup)
69+
pool/ (existing — unchanged structure)
70+
pool.odin
71+
doc.odin
72+
examples/
73+
tests/ (functional black-box tests: mbox + loop_mbox)
74+
pool_tests/ (functional black-box tests: pool — existing)
75+
design/
76+
```
77+
78+
Dependency graph:
79+
```
80+
mpsc/ — no deps
81+
wakeup/ — core:sync
82+
pool/ — core:mem, core:sync, intrusive/list, wakeup/
83+
mbox/ — core:sync, intrusive/list
84+
loop_mbox/ — mpsc/, wakeup/
85+
nbio_mbox/ — loop_mbox/, wakeup/, core:nbio
86+
```
87+
88+
---
89+
90+
## Stages
91+
92+
### Stage 1 — init returns error
93+
94+
**Claim**: `init_loop_mailbox` should return `Loop_Mailbox_Error`.
95+
Initialization can fail (`loop == nil`, keepalive timer returns nil).
96+
97+
**Verdict**: Valid. Current init ignores `nbio.timeout` return value silently.
98+
99+
**Proposal** (interim — superseded by Stage 6 final form):
100+
101+
```odin
102+
Loop_Mailbox_Error :: enum {
103+
None,
104+
Invalid_Loop,
105+
Keepalive_Failed,
106+
}
107+
108+
init_loop_mailbox :: proc(
109+
m: ^Loop_Mailbox($T),
110+
loop: ^nbio.Event_Loop,
111+
) -> Loop_Mailbox_Error
112+
where intrinsics.type_has_field(T, "node"),
113+
intrinsics.type_field_type(T, "node") == list.Node {
114+
if loop == nil { return .Invalid_Loop }
115+
m.loop = loop
116+
m.keepalive = nbio.timeout(time.Hour * 24, _noop, loop)
117+
if m.keepalive == nil { return .Keepalive_Failed }
118+
return .None
119+
}
120+
```
121+
122+
**Breaking change**: all callers handle error return.
123+
Files: `tests/loop_test.odin` (4 tests), `examples/negotiation.odin`.
124+
125+
**Design doc**: `design/loop-mbox-enhancement.md` — Stage 1 section.
126+
127+
---
128+
129+
### Stage 2 — Verify close_loop keepalive handling
130+
131+
**Claim**: `close_loop` must remove keepalive outside the mutex.
132+
133+
**Verdict**: Already correct. Current implementation matches exactly.
134+
No code change. Document and close.
135+
136+
**Design doc**: `design/loop-mbox-enhancement.md` — Stage 2 section.
137+
138+
---
139+
140+
### Stage 3 — mpsc/ package (Vyukov queue)
141+
142+
**Claim**: Replace mutex + intrusive list with lock-free Vyukov MPSC queue.
143+
144+
**Verdict**: Valid with caveats:
145+
1. `stub: list.Node` embedded — Queue NOT copyable after init
146+
2. Stall state: `pop` returns nil mid-push — retry on next tick
147+
3. No `len` counter — remove `stats` proc from `loop_mbox`
148+
4. `close_loop` drain: repeated `pop`, rebuild `list.List` for return value
149+
5. `closed` flag must use `intrinsics.atomic_load/store`
150+
151+
**New files**:
152+
```
153+
mpsc/queue.odin — Queue($T), init/push/pop
154+
mpsc/doc.odin
155+
mpsc/queue_test.odin — unit tests (package mpsc, white-box)
156+
```
157+
158+
```odin
159+
package mpsc
160+
161+
Queue :: struct($T: typeid) {
162+
head: ^list.Node, // atomic — producer side
163+
tail: ^list.Node, // consumer side only
164+
stub: list.Node, // sentinel; NOT copyable after init
165+
}
166+
// init, push (multi-producer safe), pop (single consumer only)
167+
```
168+
169+
**Updated `Loop_Mailbox` after Stage 3** (still in root, pre-Stage 6):
170+
```odin
171+
Loop_Mailbox :: struct($T: typeid) {
172+
queue: mpsc.Queue(T),
173+
loop: ^nbio.Event_Loop,
174+
keepalive: ^nbio.Operation,
175+
closed: bool, // atomic
176+
}
177+
```
178+
179+
**Design doc**: `design/loop-mbox-enhancement.md` — Stage 3 section.
180+
181+
---
182+
183+
### Stage 4 — wakeup/ package (WakeUper interface + sema impl)
184+
185+
**Claim**: Separate the wakeup mechanism from the queue.
186+
187+
**Verdict**: Valid. WakeUper is a value type (proc fields + rawptr) — copyable.
188+
`loop_mbox` stores a copy and owns it (calls `close` in `close_loop`).
189+
190+
**New files**:
191+
```
192+
wakeup/wakeup.odin — WakeUper struct + sema_wakeup
193+
wakeup/doc.odin
194+
wakeup/wakeup_test.odin — unit tests (package wakeup, white-box)
195+
```
196+
197+
```odin
198+
package wakeup
199+
200+
WakeUper :: struct {
201+
ctx: rawptr,
202+
wake: proc(rawptr),
203+
close: proc(rawptr),
204+
}
205+
206+
// sema_wakeup: WakeUper backed by sync.Semaphore.
207+
// Useful for non-nbio loops and unit tests.
208+
sema_wakeup :: proc(allocator := context.allocator) -> WakeUper
209+
```
210+
211+
**Updated `Loop_Mailbox` after Stage 4** (still in root, pre-Stage 6):
212+
```odin
213+
Loop_Mailbox :: struct($T: typeid) {
214+
queue: mpsc.Queue(T),
215+
waker: wakeup.WakeUper,
216+
closed: bool, // atomic
217+
}
218+
219+
Loop_Mailbox_Error :: enum { None, Invalid_Waker }
220+
221+
init_loop_mailbox :: proc(m: ^Loop_Mailbox($T), w: wakeup.WakeUper) -> Loop_Mailbox_Error
222+
```
223+
224+
**Design doc**: `design/loop-mbox-enhancement.md` — Stage 4 section.
225+
226+
---
227+
228+
### Stage 5 — Pool WakeUper
229+
230+
**Purpose**:
231+
Add optional `wakeup.WakeUper` to the pool so event-loop callers can be notified
232+
when a message returns to an empty pool, instead of blocking on `sync.Cond`.
233+
234+
**Depends on**: Stage 4 (`wakeup/` package must exist).
235+
236+
**Behavior** (additive — Cond blocking unchanged):
237+
- `get(.Pool_Only, 0)` returns `.Pool_Empty` and sets `p.empty_was_returned = true`.
238+
- `get(.Pool_Only, timeout<0 or >0)` still blocks on `sync.Cond` (unchanged).
239+
- `put` signals `sync.Cond` (unchanged), then if `empty_was_returned == true` and `waker.wake != nil`: calls `waker.wake(waker.ctx)`, clears flag.
240+
- `destroy` calls `cond_broadcast` (unchanged), then if waker set: `waker.wake(waker.ctx)` then `waker.close(waker.ctx)`.
241+
- WakeUper is optional. Pass `{}` to `init` for none.
242+
243+
**Pool struct changes**:
244+
```odin
245+
Pool :: struct($T: typeid) {
246+
// ... existing fields ...
247+
waker: wakeup.WakeUper, // optional — notify non-blocking callers
248+
empty_was_returned: bool, // set when get(.Pool_Only,0) found empty
249+
}
250+
```
251+
252+
**`init` change**: add `waker: wakeup.WakeUper` parameter after `reset`.
253+
254+
**-vet workaround**: add `@(private) _PoolWaker :: wakeup.WakeUper` to `pool.odin`.
255+
256+
**Files changed**:
257+
- `pool/pool.odin` — add `waker`, `empty_was_returned`; update `init`/`get`/`put`/`destroy`
258+
- `pool_tests/pool_test.odin` — add tests: WakeUper wakes on put, waker.close on destroy
259+
260+
**Design doc**: `design/loop-mbox-enhancement.md` — Stage 5 section.
261+
262+
---
263+
264+
### Stage 6 — Repartitioning
265+
266+
**Purpose**:
267+
- Move `mbox.odin` → `mbox/mbox.odin` (Mailbox only, no mpsc/wakeup deps)
268+
- Move `loop_mbox.odin` → `loop_mbox/loop_mbox.odin` (Loop_Mailbox — own package)
269+
- Move `doc.odin` → `mbox/doc.odin`; create `loop_mbox/doc.odin`
270+
- Create `nbio_mbox/` package: nbio WakeUper impl + convenience init
271+
- `mbox/` loses all nbio imports
272+
- `loop_mbox/` has no nbio dependency
273+
- Update all import paths (breaking change for callers)
274+
- Existing `tests/` updates: split or retarget to new package paths
275+
- Update examples, CI scripts, build scripts
276+
277+
**nbio_mbox/ API**:
278+
```odin
279+
package nbio_mbox
280+
281+
// wakeup returns a WakeUper backed by a 24h keepalive timer.
282+
wakeup :: proc(loop: ^nbio.Event_Loop) -> (wakeup_pkg.WakeUper, bool)
283+
284+
// init_loop_mailbox: convenience — wakeup + loop_mbox.init in one call.
285+
init_loop_mailbox :: proc(
286+
m: ^loop_mbox.Loop_Mailbox($T),
287+
loop: ^nbio.Event_Loop,
288+
) -> loop_mbox.Loop_Mailbox_Error
289+
```
290+
291+
**Design doc**: `design/loop-mbox-enhancement.md` — Stage 6 section.
292+
293+
---
294+
295+
### Stage 7 — Re-branding
296+
297+
**Purpose**:
298+
Repo name `odin-mbox` and description no longer match the library scope.
299+
Update repo identity — name, description, tagline, root README intro — to
300+
reflect a multi-package concurrency library, not just a mailbox.
301+
Details TBD when we reach this stage.
302+
303+
---
304+
305+
### Stage 8 — Documentation
306+
307+
**Purpose**:
308+
Each package is a usable standalone library.
309+
Add per-package `doc.odin` and documentation visible on GitHub.
310+
Overhaul root README as a library index.
311+
Covers: `mbox/`, `loop_mbox/`, `mpsc/`, `wakeup/`, `pool/`, `nbio_mbox/`.
312+
313+
**Willings (do not forget)**:
314+
- Per-package `README.md` with short usage snippets (visible on GitHub without cloning)
315+
- Root README becomes index linking to per-package READMEs
316+
317+
Details TBD when we reach this stage.
318+
319+
---
320+
321+
---
322+
323+
## Per-stage finish steps
324+
325+
1. Full regression — all packages build + all test suites pass
326+
2. Update `design/loop-mbox-enhancement.md`
327+
3. Sync README and docs/README
328+
4. Comments check, AI-ish scan
329+
5. Update `design/STATUS.md` checkpoint
330+
6. Update `last_plan.md` (full plan)
331+
332+
---
333+
334+
## Critical files
335+
336+
| File | Stage | Action |
337+
|------|-------|--------|
338+
| `loop_mbox.odin` | 1,3,4 | Modify (still in root until Stage 6) |
339+
| `tests/loop_test.odin` | 1,4,6 | Update init calls + imports |
340+
| `examples/negotiation.odin` | 1,4,6 | Update init call + imports |
341+
| `mpsc/queue.odin` | 3 | NEW |
342+
| `mpsc/doc.odin` | 3 | NEW |
343+
| `mpsc/queue_test.odin` | 3 | NEW (unit tests) |
344+
| `wakeup/wakeup.odin` | 4 | NEW |
345+
| `wakeup/doc.odin` | 4 | NEW |
346+
| `wakeup/wakeup_test.odin` | 4 | NEW (unit tests) |
347+
| `pool/pool.odin` | 5 | Modify (add waker, empty_was_returned) |
348+
| `pool_tests/pool_test.odin` | 5 | Add WakeUper tests |
349+
| `mbox.odin` → `mbox/mbox.odin` | 6 | MOVE |
350+
| `loop_mbox.odin` → `loop_mbox/loop_mbox.odin` | 6 | MOVE |
351+
| `doc.odin` → `mbox/doc.odin` | 6 | MOVE |
352+
| `loop_mbox/doc.odin` | 6 | NEW |
353+
| `nbio_mbox/nbio_mbox.odin` | 6 | NEW |
354+
| `nbio_mbox/doc.odin` | 6 | NEW |
355+
| `design/loop-mbox-enhancement.md` | all | NEW (add to .gitignore) |
356+
| `design/STATUS.md` | all | Update |
357+
| `last_plan.md` | all | Update |
358+
359+
---
360+
361+
## Checkpoints
362+
363+
```
364+
# Stages 1-5 (root still has mbox files):
365+
odin build . -build-mode:lib -vet -strict-style -o:none -debug
366+
odin build ./mpsc/ -build-mode:lib -vet -strict-style -o:none -debug (Stage 3+)
367+
odin build ./wakeup/ -build-mode:lib -vet -strict-style -o:none -debug (Stage 4+)
368+
odin build ./pool/ -build-mode:lib -vet -strict-style -o:none -debug
369+
odin build ./examples/ -build-mode:lib -vet -strict-style -o:none -debug
370+
odin test ./tests/ -vet -strict-style -disallow-do -o:none -debug
371+
odin test ./pool_tests/ -vet -strict-style -disallow-do -o:none -debug
372+
odin test ./mpsc/ -vet -strict-style -disallow-do -o:none -debug (Stage 3+, unit)
373+
odin test ./wakeup/ -vet -strict-style -disallow-do -o:none -debug (Stage 4+, unit)
374+
375+
# Stage 6+ (packages moved):
376+
odin build ./mbox/ -build-mode:lib -vet -strict-style -o:none -debug
377+
odin build ./loop_mbox/ -build-mode:lib -vet -strict-style -o:none -debug
378+
odin build ./nbio_mbox/ -build-mode:lib -vet -strict-style -o:none -debug
379+
odin test ./mbox/ -vet -strict-style -disallow-do -o:none -debug (unit)
380+
odin test ./loop_mbox/ -vet -strict-style -disallow-do -o:none -debug (unit)
381+
odin test ./nbio_mbox/ -vet -strict-style -disallow-do -o:none -debug (unit)
382+
odin test ./tests/ -vet -strict-style -disallow-do -o:none -debug (functional)
383+
odin test ./pool_tests/ -vet -strict-style -disallow-do -o:none -debug (functional)
384+
```

0 commit comments

Comments
 (0)