From 594a42b54e0c00fc620f2ade3a34fc1970e9bbac Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Thu, 17 Sep 2026 22:31:58 +0700 Subject: [PATCH] A map you can take a key out of, and the run closes behind it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (map-remove! m k) answers the value that was there, or None, which is the answer get already gives and for the same reason: a key that is not in the map is an answer, not a failure. Handing the value back rather than dropping it makes "take this out and use it" one call instead of two that hash the key twice. The removal shifts the probe run back over the hole. A Robin Hood lookup stops at the first empty slot, so a hole left in the middle of a run hides every entry after it — and the hidden ones are precisely what a test that only asks after what it removed never looks at, which is why the program removes a thousand of two thousand keys and then asks for the other thousand. Odin was read rather than recalled here, and it does the opposite: its erase marks a tombstone and its insert carries the repair loop. Staying tombstone- free keeps the shape the rest of the file already assumed, and the lookups — which outnumber the removals — pay nothing for it. The note in the runtime and the two in BUILT.md that said Odin deletes by backward shift were describing Odin's insert, and now say which is which. It allocates nothing and releases nothing, so there is no guard around it and it means the same thing on a map in an arena as on one in the heap: a key and a value live inside the one block the map allocated, and there was never anything per entry to hand back. --- docs/BUILT.md | 18 +++-- lib/check.ml | 58 +++++++++++++++- lib/emit.ml | 1 + runtime/flan_rt.c | 87 +++++++++++++++++++++--- test/programs/map-remove.flan | 121 ++++++++++++++++++++++++++++++++++ test/test_acceptance.ml | 24 +++++++ 6 files changed, 294 insertions(+), 15 deletions(-) create mode 100644 test/programs/map-remove.flan diff --git a/docs/BUILT.md b/docs/BUILT.md index ddf6446..aa83119 100644 --- a/docs/BUILT.md +++ b/docs/BUILT.md @@ -2599,9 +2599,14 @@ Odin's header states them and they are why it was the thing to follow (`base/run ### Two departures from Odin, both deliberate -**There are no tombstones**, because `spec-memory.md` defers removal ("Move-aware lookup, removal, and owned entries -are deferred"). A slot is empty or occupied and nothing else, which deletes Odin's backward-shift loop entirely — it -is the single largest reason this file is shorter than the original. When removal arrives, that loop is what it costs. +**There are still no tombstones, now that removal exists.** A slot is empty or occupied and nothing else, and +`flan_map_remove` keeps it that way by shifting the run back over the hole rather than marking it. Odin does the +opposite, and this paragraph used to say otherwise: read out of +`base/runtime/dynamic_map_internal.odin`, `map_erase_dynamic` sets a tombstone bit and leaves the repair to the next +insert, which is why Odin's *insert* carries a backward-shift loop and its every lookup tests for a tombstone. The +trade is the usual one — erase is O(1) there and the shift is here, and the lookups, which outnumber the removals, +pay nothing. What `spec-memory.md` still defers is the rest of its sentence: move-aware lookup and owned entries. A +removed value is copied out, and nothing is dropped. **The header does not tag the capacity into the data pointer.** Odin stuffs `log2cap` into the low six bits because its `Raw_Map` must be three words. This header already carries an allocator, a generation and an epoch, so the tagging @@ -2649,6 +2654,7 @@ calls per field, which has no channel to hand on. | `(put m k v)` | upsert, `()` | | `(get m k)` | `(Option V)` — absence is `None` | | `(has-key? m k)` | `bool`, copying no value — **an addition; the spec does not name it** | +| `(map-remove! m k)` | `(Option V)` — the value that was there, or `None` | | `(len m)` `(reserve m n)` `(clone m)` `(clone m a)` `(free m)` | extended, not duplicated | `has-key?` is **not in `spec-memory.md`** and is an addition, flagged because everything else here is the spec's. @@ -3743,8 +3749,10 @@ function. ``` **The cursor is a slot index the caller owns, and there is no iterator struct** because there is nothing for one to -hold. A map has no tombstones — removal is deferred (`spec-memory.md`) — so a slot is either empty or occupied and the -position is the whole of the state. The cursor starts at 0, comes back one past the entry just answered, and is left +hold. A map has no tombstones — removal shifts the run back instead of marking a hole — so a slot is either empty or +occupied and the position is the whole of the state. What a cursor does *not* survive is a removal taken while it is +in flight: the shift moves entries to lower slots, and a cursor already past them steps over entries it has not +answered, the same bargain a put that grows already makes. The cursor starts at 0, comes back one past the entry just answered, and is left at `cap` by the call that answers false, so a spent cursor keeps answering false rather than wrapping. **Three out-pointers and not a returned pair**, because there are no tuples. An `(Option K)` would answer half an diff --git a/lib/check.ml b/lib/check.ml index 3b140f2..8411eb2 100644 --- a/lib/check.ml +++ b/lib/check.ml @@ -4527,6 +4527,62 @@ and named_call ctx ~want loc name args = [ mk loc oty (Tast.If (cond, some, none)) ]))) | _ -> assert false) + (* (map-remove! m k) -> (Option V): the value that was there, or None when + the key was not. The same answer [get] gives, for the same reason — a key + that is not in the map is an answer and not a failure — and the value + comes back rather than being dropped on the floor, which is what makes + "take this out and use it" one call instead of a get and a remove that + hash the key twice. + + It allocates nothing and releases nothing, so unlike [put] there is no + alloc_guard and no region check around it: a key and a value live inside + the one block the map allocated, and removal moves entries within that + block. That is what makes it mean the same thing on a map backed by an + arena — or by any allocator that refuses can-free — as on a heap-backed + one. Nothing is freed per entry because nothing was allocated per entry. + + The [!] is the mutation the naming rule asks for ([map-next!], and the + note below on the two suffixes). *) + | "map-remove!" -> + arity loc name 2 args; + (match args with + | [ target; k ] -> + let target = borrowed ctx target (fun () -> check ctx target) in + let kt, vt = map_kv loc "map-remove!" target.Tast.ty in + let k = check ctx ~want:kt k in + (* Deferred exactly as [get] is, and with [None] for the same reason: + the abstract pass still has to check whatever the body does with the + answer. *) + if deferred_key ctx.env loc "map-remove!" kt then + expect loc ~want (mk loc (Types.Option vt) Tast.None_) + else + let hash, eq = key_fns ctx.env loc kt in + let ks = fresh_slot ctx kt in + let out = fresh_slot ctx vt in + let found = + rt loc (Types.Int Types.I8) "flan_map_remove" + [ target; addr_of loc (mk loc kt (Tast.Local ks)); + addr_of loc (mk loc vt (Tast.Local out)); + size_of loc kt; size_of loc vt; hash; eq; here loc ] + in + let oty = Types.Option vt in + (* Built here and not there, as [get]'s is: the runtime fills [out] only + when it answers 1 and has no idea what an Option's layout is. *) + let some = mk loc oty (Tast.Some_ (mk loc vt (Tast.Local out))) in + let none = mk loc oty Tast.None_ in + let cond = + mk loc Types.Bool + (Tast.Prim (Tast.Ne, + [ found; + mk loc (Types.Int Types.I8) (Tast.Int (0L, Types.I8)) ])) + in + expect loc ~want + (mk loc oty + (Tast.Let ([ (ks, k); + (out, mk loc vt (Tast.Zero vt)) ], + [ mk loc oty (Tast.If (cond, some, none)) ]))) + | _ -> assert false) + (* (map-next! m (addr cur) (addr k) (addr v)) -> bool, and the whole of map iteration. Before it there was no way to read a map's keys or its values at all: every other map operation addresses one entry by hashing it, and @@ -5047,7 +5103,7 @@ and named_call ctx ~want loc name args = call site these can be refused at. They are deferred and then always succeed. That is the cheapest possible membership. - The map operations — [put], [get], [has-key?], [reserve], [clone], + The map operations — [put], [get], [has-key?], [map-remove!], [reserve], [clone], through [deferred_key] beside [key_fns] — are the other kind, and they are here on a different argument. They *can* fail at a concrete type, so deferring them does move a refusal. But [{:where (hashable? $t)}] is diff --git a/lib/emit.ml b/lib/emit.ml index 5ef246b..4f72192 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -2782,6 +2782,7 @@ declare i8 @flan_map_init(ptr, ptr, i64, i64, ptr, i64) declare i8 @flan_map_put(ptr, ptr, ptr, i64, i64, ptr, ptr, ptr, i64) declare i8 @flan_map_get(ptr, ptr, ptr, i64, i64, ptr, ptr, ptr, i64) declare i8 @flan_map_has(ptr, ptr, i64, i64, ptr, ptr, ptr, i64) +declare i8 @flan_map_remove(ptr, ptr, ptr, i64, i64, ptr, ptr, ptr, i64) declare i8 @flan_map_reserve(ptr, i64, i64, i64, ptr, ptr, i64) declare i8 @flan_map_clone(ptr, ptr, ptr, i64, i64, ptr, ptr, i64) declare i64 @flan_map_len(ptr, ptr, i64) diff --git a/runtime/flan_rt.c b/runtime/flan_rt.c index 5f0ffac..30e8a7f 100644 --- a/runtime/flan_rt.c +++ b/runtime/flan_rt.c @@ -1873,11 +1873,14 @@ void flan_pool_free(flan_pool *p, int64_t size, int64_t align, #define FLAN_MAP_MIN_LOG2 3 /* 8 slots */ /* The hash word. Zero means the slot is empty, which is what makes a - * zeroed hash block an empty map. There is no tombstone: removal is deferred - * (spec-memory.md defers move-aware lookup, removal and owned entries), so the - * only two states a slot has are empty and occupied. That deletes Odin's - * backward-shift loop from this file entirely, and it is the single largest - * reason this is shorter than the Odin original. + * zeroed hash block an empty map. There is still no tombstone now that + * removal exists: the only two states a slot has are empty and occupied, and + * flan_map_remove restores that by shifting the run back rather than by + * marking the hole. Odin marks it and repairs on the next insert, which is + * why its insert has a second loop and its every lookup tests for a + * tombstone; neither is here. What spec-memory.md still defers is the rest of + * that sentence — move-aware lookup and owned entries — so a removed value is + * copied out and nothing is dropped. * * The top bit is set on every stored hash so that a hasher answering 0 does * not read as an empty slot. It is the highest bit, so the desired slot and @@ -2532,6 +2535,69 @@ int8_t flan_map_has(flan_map *m, const void *key, int64_t ksize, int64_t vsize, return (int8_t)(flan_map_find_g(m, key, ksize, vsize, hash, eq, NULL) >= 0); } +/* Removal, by backward shift, which is what keeps this file tombstone-free. + * + * Odin's own erase (base/runtime/dynamic_map_internal.odin, map_erase_dynamic) + * marks a tombstone and leaves the repair to the next insert, which is why its + * insert carries a second loop this file has never had. Read rather than + * recalled: the note in this file that said Odin deletes by backward shift was + * describing its *insert*. The trade is the usual one — erase is O(1) there + * and the shift is here, and every lookup there pays a tombstone test this one + * does not. + * + * The invariant Robin Hood lookups depend on is that no live element is ever + * separated from its home slot by an empty one: the probe stops at the first + * empty slot, so a hole left in the middle of a run would hide everything + * after it. So the hole walks forward: each following element that is not + * already home moves back one slot, and the walk stops at the first slot that + * is empty or whose occupant is already home — neither can be moved back, and + * neither can be hiding anything. + * + * It releases nothing. A key and a value live inside the one block the map + * allocated, so there is no per-entry allocation to hand back and nothing here + * asks the allocator for anything — which is what makes removal from a map + * backed by an arena, or by any allocator that refuses can-free, mean exactly + * what it means for a heap-backed one. The block is released only by free and + * by the grow that replaces it. + * + * [out] takes a copy of the value that was there, or is NULL when the caller + * does not want one. A cursor held across this is invalidated the way a put + * that grows invalidates one: the shift moves entries to lower slots, and an + * iteration resuming at a higher index would step over them. */ +int8_t flan_map_remove(flan_map *m, const void *key, void *out, int64_t ksize, + int64_t vsize, flan_hash_fn hash, flan_eq_fn eq, + const uint8_t *loc, int64_t loclen) { + flan_map_geom g; + int64_t at, mask, pos; + flan_map_check(m, loc, loclen); + at = flan_map_find_g(m, key, ksize, vsize, hash, eq, &g); + if (at < 0) return 0; + if (vsize > 0 && out) + flan_copy_small( + out, flan_cell_at(g.vs, vsize, g.vepc, g.vcell, g.vshift, at), vsize); + mask = flan_map_cap(m) - 1; + pos = at; + for (;;) { + int64_t next = (pos + 1) & mask; + uint64_t eh = g.hs[next]; + if (eh == 0 || flan_map_distance(eh, next, mask) == 0) { + g.hs[pos] = 0; + break; + } + flan_copy_small(flan_cell_at(g.ks, ksize, g.kepc, g.kcell, g.kshift, pos), + flan_cell_at(g.ks, ksize, g.kepc, g.kcell, g.kshift, next), + ksize); + if (vsize > 0) + flan_copy_small( + flan_cell_at(g.vs, vsize, g.vepc, g.vcell, g.vshift, pos), + flan_cell_at(g.vs, vsize, g.vepc, g.vcell, g.vshift, next), vsize); + g.hs[pos] = eh; + pos = next; + } + m->len--; + return 1; +} + int64_t flan_map_len(flan_map *m, const uint8_t *loc, int64_t loclen) { flan_map_check(m, loc, loclen); return m->len; @@ -2548,13 +2614,16 @@ int64_t flan_map_len(flan_map *m, const uint8_t *loc, int64_t loclen) { * slot index gives for free: it starts at 0, it is written back one past the * entry just answered, and a 0 answer leaves it at [cap] so calling again is * still 0. There is no iterator struct because there is nothing for one to - * hold — a map has no tombstones (removal is deferred), so no state beyond the - * position is needed to know where to resume. + * hold — a map has no tombstones, so no state beyond the position is needed to + * know where to resume. * * Invalidated by anything that moves the block, exactly as a Vec's slice is: * a put that grows rehashes into a new block and every index before it means a - * different entry. The epoch check below catches a released arena and nothing - * catches a resize, which is the same bargain [as-slice] already makes. + * different entry. A remove is the same hazard without the reallocation — its + * backward shift moves entries to lower slots, and a cursor already past them + * steps over entries it has not answered. The epoch check below catches a + * released arena and nothing catches either of these, which is the same + * bargain [as-slice] already makes. * * The layout is the one the geometry describes and is worth restating because * it is the thing most likely to be got wrong here: [data] is *one* allocation diff --git a/test/programs/map-remove.flan b/test/programs/map-remove.flan new file mode 100644 index 0000000..85223c8 --- /dev/null +++ b/test/programs/map-remove.flan @@ -0,0 +1,121 @@ +;;;; (map-remove! m k) — the operation a Map has been missing. +;;;; +;;;; Removal is the one map operation that can break the *other* ones: Robin +;;;; Hood lookups stop at the first empty slot, so a hole punched in the middle +;;;; of a probe run hides every entry after it, and the entries it hides are +;;;; found by no test that only removes and asks about what it removed. So the +;;;; rows below are mostly about the survivors. +;;;; +;;;; Odin marks a tombstone and repairs on the next insert; this shifts the run +;;;; back and stays tombstone-free, which is the arrangement the rest of the +;;;; file already assumed. A version that punched the hole and left it passes +;;;; row 1 and fails row 3. +(defstruct Cell [x i32 y i32]) + +(defn main [] i32 + ;; (1) The value comes back, the length drops, and the key is gone. An + ;; absent key is None and changes nothing — the same answer get gives, since + ;; a key that was not there is an answer and not a failure. + (let [m (map-new i32 i64)] + (put m 1 100) + (put m 2 200) + (match (map-remove! m 1) + (Some v) (do (print v) (println "")) ; 100 + None (println "missing")) + (print (len m)) (println "") ; 1 + (print (has-key? m 1)) (println "") ; false + (match (map-remove! m 1) (Some v) (do (print v) (println "")) None (println "gone")) + (println (len m)) ; gone, then 1 + (free m)) + + ;; (2) Removing every entry empties the map, and it is usable afterwards: + ;; the block is still there and the slots are empty rather than poisoned. + (let [m (map-new i32 i32)] + (dotimes [i 64] (put m i i)) + (dotimes [i 64] (map-remove! m i)) + (print (len m)) (println "") ; 0 + (put m 7 77) + (match (get m 7) (Some v) (do (print v) (println "")) None (println "?")) ; 77 + (print (len m)) (println "") ; 1 + (free m)) + + ;; (3) The survivors, which is the row that matters. 2000 entries is eight + ;; grows, so the runs are long and interleaved; removing the even keys and + ;; then asking after every odd one is asking whether any probe run was cut. + ;; A backward shift that stopped one slot early loses entries here and + ;; nowhere else. + (let [m (map-new i32 i64)] + (dotimes [i 2000] (put m i (* (i64 i) 3))) + (let [taken 0] + (dotimes [i 2000] + (if (= 0 (% i 2)) + (match (map-remove! m i) + (Some v) (if (= v (* (i64 i) 3)) (set taken (+ taken 1))) + None (set taken taken)))) + (print taken) (println "")) ; 1000 + (print (len m)) (println "") ; 1000 + (let [lost 0 ghosts 0] + (dotimes [i 2000] + (match (get m i) + (Some v) (if (or (= 0 (% i 2)) (not (= v (* (i64 i) 3)))) + (set ghosts (+ ghosts 1))) + None (if (not (= 0 (% i 2))) (set lost (+ lost 1))))) + (print lost) (println "") ; 0 + (print ghosts) (println "")) ; 0 + ;; Re-inserting what was taken out puts the length back, through no grow: + ;; the block still has the room the removals freed up. + (dotimes [i 2000] (if (= 0 (% i 2)) (put m i (* (i64 i) 3)))) + (print (len m)) (println "") ; 2000 + (free m)) + + ;; (4) A struct key and a string key, so the emitted hash/equality pair is + ;; on the removal path too and not only on get's. + (let [g (map-new Cell i32)] + (dotimes [i 20] (dotimes [j 20] (put g (Cell {.x i .y j}) (+ (* i 100) j)))) + (match (map-remove! g (Cell {.x 7 .y 9})) + (Some v) (do (print v) (println "")) ; 709 + None (println "?")) + (print (has-key? g (Cell {.x 7 .y 9}))) (println "") ; false + (print (has-key? g (Cell {.x 7 .y 10}))) (println "") ; true + (print (len g)) (println "") ; 399 + (free g)) + + (let [s (map-new string i32)] + (put s "alpha" 1) + (put s "beta" 2) + (match (map-remove! s "alpha") + (Some v) (do (print v) (println "")) ; 1 + None (println "?")) + (print (has-key? s "beta")) (println "") ; true + (print (len s)) (println "") ; 1 + (free s)) + + ;; (5) Iteration after removals answers exactly the survivors. The cursor is + ;; started fresh — a cursor held *across* a removal is invalidated by the + ;; shift, the same way a put that grows invalidates one. + (let [m (map-new i32 i32)] + (dotimes [i 100] (put m i i)) + (dotimes [i 100] (if (= 0 (% i 3)) (map-remove! m i))) + (let [cur (i64 0) k 0 v 0 seen 0 sum 0] + (while (map-next! m (addr cur) (addr k) (addr v)) + (set seen (+ seen 1)) + (if (not (= k v)) (set sum (+ sum 1)))) + (print seen) (println "") ; 66 + (print sum) (println "")) ; 0 + (print (len m)) (println "") ; 66 + (free m)) + + ;; (6) An arena, which refuses can-free. Removal asks the allocator for + ;; nothing and hands it back nothing — a key and a value live inside the one + ;; block the map allocated — so it means here exactly what it means on the + ;; heap, and the region is released whole as always. + (let [ar (arena-new 1048576)] + (with-allocator ar + (let [t (map-new i32 i32)] + (dotimes [i 300] (put t i (* i 2))) + (dotimes [i 300] (if (= 0 (% i 2)) (map-remove! t i))) + (print (len t)) (println "") ; 150 + (match (get t 299) (Some v) (do (print v) (println "")) None (println "?")) ; 598 + (print (has-key? t 298)) (println ""))) ; false + (free-all ar)) + 0) diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index 149907d..b679aa8 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -2413,6 +2413,25 @@ ERR@7 unexpected token: not the kind the caller was reading outputs "map iteration" "programs/map-iter.flan" map_iter_out; outputs ~opt:"-O0" "map iteration, -O0" "programs/map-iter.flan" map_iter_out; + (* Removal, which is the operation that can break the others. A Robin Hood + probe stops at the first empty slot, so a hole left in the middle of a + run hides every entry past it — and the hidden ones are exactly what a + test that only asks after what it removed never looks at. Hence the + third row: 2000 entries, the even keys taken out, and then every odd one + asked for. A removal that punched the hole and left it answers row one + correctly and loses entries there. + + -O0 as well, because the Option the removal answers with is built in the + compiler and not in the runtime, and mem2reg is what would hide a store + into the wrong half of it. *) + let map_remove_out = + "100\n1\nfalse\ngone\n1\n0\n77\n1\n1000\n1000\n0\n0\n2000\n\ + 709\nfalse\ntrue\n399\n1\ntrue\n1\n66\n0\n66\n150\n598\nfalse\n" + in + outputs "map removal" "programs/map-remove.flan" map_remove_out; + outputs ~opt:"-O0" "map removal, -O0" "programs/map-remove.flan" + map_remove_out; + (* The allocation-failure rule is one rule over every allocating operation, so it has to hold for map-new, put, reserve and clone as it does for the Vec's four. A map is the harder case: its growth allocates a new block, @@ -2429,6 +2448,11 @@ ERR@7 unexpected token: not the kind the caller was reading is the refusal (Vec (Vec T)) already carries, for the identical reason. Unit as a value is refused rather than dividing a cache line by zero, and it is named because it is the natural spelling of a set. *) + (* Removal is a map's operation and says so, rather than reaching for a + [len] that a Vec would also answer. *) + refuses_src "map-remove! wants a map" + "(defn main [] i32 (let [v (vec-new i32)] (map-remove! v 1) (free v)) 0)" + "map-remove! takes a (Map K V)"; refuses_src "a float is not a map key" "(defn f [m (Map f32 i32)] () 0)" "is not a map key"; refuses_src "a Ptr is not a map key"