From 7c1a818001969ce2a920a9a616f336c51d27bf4f Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sat, 19 Sep 2026 21:59:11 +0700 Subject: [PATCH] The defer counter costs something on the warm path, and the valgrind row rests on one half Three notes, all of them about a comment that was true as far as it went. The cost sentence on defer_slot said one i64 and one compare on a path that is already unwinding, which is the unwind's share and not the whole bill: every function with a defer also pays a store of zero at entry and a store of an ordinal at each defer, on the ordinary path, whether anything transfers or not. Small, correct, and now written down as what it is. init-conditions.flan is in test_valgrind.ml, and only the game-data half earns that row -- it is the one that reaches a real (free src) over a slot slurp transferred out of. The note half is output-only: revert the fix and its trace changes, but a wrong integer in a global is not a memory error and memcheck stays quiet. A later edit trimming the edn dependency would leave the row green and blind, so the header says so. And register_defer saves in_defer rather than clearing it. Unreachable today, because defer_ok is false inside a defer and nothing can nest one; written so that the flag comes back rather than being dropped on the day that changes. --- lib/check.ml | 20 ++++++++++++++++---- test/programs/init-conditions.flan | 11 +++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/check.ml b/lib/check.ml index 9da69f7..6e7dd76 100644 --- a/lib/check.ml +++ b/lib/check.ml @@ -249,9 +249,15 @@ type ctx = { not a leak the other way round; it is cleanup over a binding nothing has written, which is [(free v)] on whatever the stack held. - So the count is kept at run time, one store per defer, and the transfer - path's copy of each is guarded on it. One i64 and one compare per defer - on a path that is already unwinding. *) + So the count is kept at run time and the transfer path's copy of each + defer is guarded on it. The cost is not all on the unwinding path, and + the half that is not is the half worth naming: every function with a + defer pays one i64 of frame, one store of zero at entry, and one store + of an ordinal where each defer is written — on the ordinary path, + whether anything ever transfers or not. What the unwind adds on top is + one compare per defer. A store of a constant into a frame slot nothing + else reads is about as little as a fact can cost, but it is not nothing + and it is not only on the cold path. *) mutable defer_slot : int option; (* Where a [defer] may be written, which is exactly: a form whose extent is the whole function body. Two things have that extent and only two — a @@ -2732,6 +2738,12 @@ and check_handler_case ctx ?want loc body clauses = has registered from one the text has not reached yet. The form's type is still [unit], which is all a reader of the value can see. *) and register_defer ctx loc forms = + (* Saved and put back rather than cleared, which is the same thing today and + will not be the day a defer may hold one. Nothing reaches here from + inside a defer now — [defer_ok] is false in there — so the saved value is + always false; written this way so that if it ever is not, the flag comes + back rather than being dropped. *) + let was_in_defer = ctx.in_defer in ctx.in_defer <- true; (* A barrier, for the reason [defer] itself exists: these forms are *copied* into every exit path of the function, where the loop they were written @@ -2740,7 +2752,7 @@ and register_defer ctx loc forms = let forms = barrier ctx "a defer" (fun () -> map_lr (fun d -> check ctx d) forms) in - ctx.in_defer <- false; + ctx.in_defer <- was_in_defer; ctx.defers <- mk loc Types.Unit (Tast.Do forms) :: ctx.defers; let slot = match ctx.defer_slot with diff --git a/test/programs/init-conditions.flan b/test/programs/init-conditions.flan index 65c613a..ee56bd2 100644 --- a/test/programs/init-conditions.flan +++ b/test/programs/init-conditions.flan @@ -28,6 +28,17 @@ ;;;; ;;;; [log] is a digit trace and not a running sum, for handler-case.flan's ;;;; reason: a sum commutes and would score a wrong order right. +;;;; +;;;; DO NOT DROP THE edn IMPORT. This program is in test_valgrind.ml, and only +;;;; one half of it earns that row: [game-data] below, whose initialiser goes +;;;; through edn/read-file and so through the real (defer (free src)) over a +;;;; binding slurp transferred out of. That is the free of an uninitialised +;;;; slot, and memcheck names it. The [note] half is output-only — revert the +;;;; fix and its printed trace changes, but no memcheck error is produced, +;;;; because a wrong integer written to a global is not a memory error. So a +;;;; later edit that trims the edn dependency out of here leaves the valgrind +;;;; row green and no longer looking at anything. Move it to a program that +;;;; still frees, or take the row out honestly. (import edn "vendor:edn")