diff --git a/BUILT.md b/BUILT.md index 7f19e5e..c7b728e 100644 --- a/BUILT.md +++ b/BUILT.md @@ -290,6 +290,71 @@ importing at build time**, on the branch where the dev loop is the priority. It is the strongest case for the third option — generate from the header, commit the result, regenerate when the library moves — and that decision is open. +#### Where that 15.5ms actually is — and it is not the header + +Written above as though a redefinition were re-reading the header on every +reload. It is not, and the number was never taken apart. Taken apart now, with a +timer around each stage of the same `flan reload` (raylib 5.5, sand.flan, one +`defn` in the forms file, warm caches): + +| | no header | header imported | delta | +|---|---|---|---| +| `Session.create` | 7.0ms | 21.5ms | +14.5 | +| `Session.eval` | 4.8ms | 8.4ms | +3.6 | +| `llc` + `ld` | 21ms | 21ms | 0 | + +And inside that `create`, the header's own share: reading the cached dump +**0.33ms**, turning it into declarations **3.3ms**, checking the package's +layouts and its hand-written signatures against it **0.55ms**. Roughly 4ms of +the 14.5, once. The other ~10ms is `Load` and `Check` doing their ordinary work +over 256 more declarations, and so is the +3.6ms on `eval`. + +**So a `C-c C-c` in a daemon pays no header cost at all, and never did.** +`Session.eval` puts the evaluated forms through `Load.program`, and a form list +with no `(import …)` in it reads no package and therefore no header. The import +runs when the session is created, and again on a `C-c C-k` whose buffer carries +the package's own `import` line. `flan reload` is a fresh process per +redefinition, which is why its number carries a session's startup cost — it is +the CLI's shape, not the dev loop's. + +What a redefinition does pay for an imported package is the +3.6ms of checking +and emitting against a program with 256 more declarations in it. That is in +`Check` and in `Emit.redefinition` declaring every sibling; it is real, it is +the number to attack next, and no cache touches it. + +#### Two caches, and what a header change mid-session means + +Both levels exist now. **On disk**, keyed on the header's realpath, size, mtime, +the full flag list and a format version — the object cache's own convention, and +it now lives in the object cache directory rather than a second one beside it. +Cold that read is 50ms of clang and parse; served from the file it is 0.33ms. + +**In the session**, two tables in `Cimport`: the extracted dump by header, and +the generated declarations by header *and* by what the package already declares +(names taken, structs and enums known, C symbols bound by hand), so a package an +evaluation has added a decl to is worked out again rather than served a stale +answer. A repeat import in one process goes from 3.65ms to nothing — measured on +a whole-buffer evaluation, which imports twice. + +**A header edited mid-session does not take effect until the session is +restarted**, and that is deliberate rather than incidental: the in-memory key is +the header's path and flags, with no mtime and no `stat`. It is the rule a +changed `.c` file already follows — the daemon compiled the package's C at +startup and a redefinition does not recompile it — and the rule the running +program follows, since its struct layouts are the ones it was built with. Taking +a new header mid-session would mean type-checking new signatures against a +process still running the old layouts, which is the silent disagreement the +whole header check exists to catch. The disk cache does key on mtime, so the +next session reads the new header rather than the old dump. + +One attribution to not repeat: the disk cache is **not** what the dev build's ++333ms cold is about. Measured on this machine, a cold dev build of sand.flan is +1.37s without the header and 1.83s with it and both caches cold; clear only the +header cache and the same build is 0.92s against 0.83s. So the header read is +~60–90ms of a fresh session and the rest of the cold delta is the object cache +compiling a shim with 428 wrappers in it, which is the object cache's business +and already warm after one build. + ### What a headless FFI test can and cannot pin Worth knowing before writing another one, because two plausible tests in a row turned out to check nothing. diff --git a/NEXT.md b/NEXT.md index ef6fc4f..2def818 100644 --- a/NEXT.md +++ b/NEXT.md @@ -62,6 +62,15 @@ a wrapper nothing reachable calls, confirmed on the wasm32 case it exists for wi Redefinition 31.0ms → 46.5ms. Dev build +333ms cold, once per session, since `Build.shared` compiles no C. Reading the header is cached (64ms → 17ms), keyed like the object cache; the cache was built against a measurement, not a guess. +**Re-measured, and the 15.5ms was misattributed** — see BUILT.md, "Where that 15.5ms actually is". A `C-c C-c` reads +no header: `Session.eval` puts the forms through `Load`, and forms with no `(import …)` in them touch no package. The +15.5ms is `flan reload`'s, and `flan reload` is a fresh process — ~14.5ms of it is session startup and ~4ms of *that* +is the header. What a redefinition really pays for an imported package is **+3.6ms per eval** in `Check` and in +`Emit.redefinition` declaring 256 more siblings, and no cache touches that; it is the number to attack next. The +header is now cached in the session as well as on disk, so a repeat import (a `C-c C-k` of a buffer carrying its own +`import` line) costs nothing, and a header edited mid-session is not picked up until the session restarts — the same +rule a changed `.c` file follows. + **Opt-in on purpose.** `vendor/raylib/headers` is `?${FLAN_RAYLIB_H}`. "A build needs libraylib linkable and not raylib-devel installed" is a property chosen deliberately, and requiring a header would take it from everyone to give the check to whoever has one. Unset means off; set-and-wrong is an error naming the path. @@ -84,8 +93,9 @@ Worth knowing before touching it: Two things that are *not* done, and are 6a and 6b in DISCUSS.md: whether the header stays a build-time read or becomes a committed generator (`flan import-c` already prints the lines, so it costs nothing more to switch), and whether the -172 hand-written lines migrate. Neither is blocked on correctness. The 15.5ms on redefinition is the argument for the -first; needing the header at every build — vendoring raylib.h or requiring raylib-devel — is the argument on the +172 hand-written lines migrate. Neither is blocked on correctness. The argument for the first is weaker than it +looked — committing the generated lines would save ~4ms of session startup and none of the +3.6ms per redefinition, +since that cost is the 256 declarations existing at all and not where they came from; needing the header at every build — vendoring raylib.h or requiring raylib-devel — is the argument on the second. One smaller thing found and worth not re-deriving: an enum parameter imports as `i32`, because the header says diff --git a/lib/cimport.ml b/lib/cimport.ml index cb5d2f8..9d14469 100644 --- a/lib/cimport.ml +++ b/lib/cimport.ml @@ -678,7 +678,7 @@ let dump_of_clang ~loc ~header ~flags = in read_dump ~header json -(* ── The cache ─────────────────────────────────────────────────────── *) +(* ── The caches, two of them ───────────────────────────────────────── *) (* Measured, not assumed: reading raylib.h costs 64ms — 30ms for clang to write 1.8 MB of JSON and the rest to parse it and map it — against an 8ms check @@ -697,16 +697,18 @@ let dump_of_clang ~loc ~header ~flags = compiler whose [dump] type has changed must not read one written by the old one. [Marshal] does not check that for you and a mismatch is a segfault rather than an exception, so the discipline is: **change the [dump] type, - bump [cache_format] in the same commit.** Nothing enforces it. *) + bump [cache_format] in the same commit.** Nothing enforces it. + + It lives under the object cache directory, beside the [.o] files, because it + is the same kind of thing: derived from an input the compiler did not write, + stable across builds, and safe to delete. One directory to clear rather than + two. *) let cache_format = 1 -let cachedir () = - let d = Filename.concat (Filename.get_temp_dir_name ()) "flan-cimport" in - (try Unix.mkdir d 0o700 with Unix.Unix_error (Unix.EEXIST, _, _) -> ()); - d +let cachedir () = Build.cachedir () -let dump_of ~loc ~header ~flags = +let dump_of_disk ~loc ~header ~flags = let st = try Some (Unix.stat header) with Unix.Unix_error _ -> None in match st with | None -> dump_of_clang ~loc ~header ~flags @@ -753,12 +755,75 @@ let dump_of ~loc ~header ~flags = with Sys_error _ -> ()); d) +(* And in front of that file, the session's own copy — which is the level that + matters for the dev loop, because the daemon is a process that lives for as + long as the editor does and re-reads the file once per evaluation that + imports the package again. Reading the cached dump is 0.33ms and extracting + the declarations from it is another 3.3ms, so this is small; it is here + because *nothing* is the right amount for a long-lived process to pay twice + for an answer it already has. + + **Keyed on the header's path and the flags, and deliberately not on its + mtime.** That is the whole of the mid-session question: a header edited + while a session is running is not re-read, and the session keeps the + signatures it started with until it is restarted. It is the same rule a + changed [.c] file follows — the daemon compiled the package's C once, at + startup, and a redefinition does not recompile it — and the same rule the + running program itself follows, since its layouts are the ones it was built + with. The alternative is worse in both directions: keying on mtime would put + a [stat] on a path that is supposed to cost nothing, and it would let a + header change take effect on the next [C-c C-c] in a *program that is still + running with the old layouts*, which is precisely the silent disagreement + the header check exists to prevent. A new session reads the new header, and + the disk cache above keys on mtime so it does not serve it the old one. + + Two tables rather than one, because the two answers have different inputs. + The *dump* is the header's alone, so a second package importing the same + header shares it. The *declarations* are the dump read against one package — + which names it already has taken, which structs and enums it knows, which C + symbols it binds by hand — so they are keyed on those too, and a package + whose decls an evaluation has added to gets its declarations worked out + again rather than served an answer about the decls it used to have. *) + +let dumps : (string, dump) Hashtbl.t = Hashtbl.create 4 +let imports : (string, imported * dump * env) Hashtbl.t = Hashtbl.create 4 + +let header_key ~header ~flags = + String.concat "\000" + ((try Unix.realpath header with Unix.Unix_error _ -> header) :: flags) + +let dump_of ~loc ~header ~flags = + let k = header_key ~header ~flags in + match Hashtbl.find_opt dumps k with + | Some d -> d + | None -> + let d = dump_of_disk ~loc ~header ~flags in + Hashtbl.replace dumps k d; + d + let env_of ~known_structs ~known_enums d = { known_structs; known_enums; d } let header ~loc ~header:h ~flags ~known_structs ~known_enums ~taken ~bound_syms = - let d = dump_of ~loc ~header:h ~flags in - let env = env_of ~known_structs ~known_enums d in - (of_dump ~env ~taken ~bound_syms d, d, env) + let k = + (* Sorted, because neither the taken table nor the declaration order is a + fact about the package — two loads of the same file that enumerate them + differently are the same question and must not miss each other. *) + let sorted xs = List.sort compare xs in + String.concat "\000" + (header_key ~header:h ~flags + :: "\001" :: sorted known_structs + @ ("\001" :: sorted known_enums) + @ ("\001" :: sorted (Hashtbl.fold (fun n () acc -> n :: acc) taken [])) + @ ("\001" :: sorted bound_syms)) + in + match Hashtbl.find_opt imports k with + | Some r -> r + | None -> + let d = dump_of ~loc ~header:h ~flags in + let env = env_of ~known_structs ~known_enums d in + let r = (of_dump ~env ~taken ~bound_syms d, d, env) in + Hashtbl.replace imports k r; + r (* ── Printing a declaration back as source ─────────────────────────── *)