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"