Skip to content

Commit 7ecb6cf

Browse files
committed
Build the path of a component found through an open with its binding
Why: Id_tbl.find_opened returned the binding and its position as a new pair, which find_name and find_all took apart again to build the path, so every lookup of a name found in the components of an open allocated three words more than before the open tables took the cells. In stress/RefsInLoop that moved each statement's allocations from 46272 to 46368 bytes, a Fibonacci number: with the minor heap the benchmarks run with, the objects of the 300 statements in the .cmt then fall on the same slot of the marshaller's position table (Fibonacci hashing of addresses, linear probing), and marshalling the .cmt took about 40M more instructions (+23% under cachegrind), though the file is byte-identical. find_opened now gives the path and the binding, built from the pair stored in the table; update, which only Env.copy_types calls, looks the binding up as before. The bindings found, their order and the outputs are unchanged. Measured with scripts/compilerBenchmarks.js against the previous commit: allocated words -0.02% in total, stress/NestedClosures -0.13%, and stress/RefsInLoop -5.4K words; with perf stat (s=64M as the benchmarks run) RefsInLoop takes 456.5M instructions, against 515.0M before and 456.1M for #9192 head. Review: Does the instruction count of RefsInLoop now depend on luck? Yes, as before #9194: any change of 96 bytes per statement in this benchmark can land on a stride that hashes badly, without any work in the compiler. The fix removes the allocation that moved it there, which is worth having on its own. Signed-off-by: Jono Prest <jjprest@gmail.com>
1 parent ddc061f commit 7ecb6cf

1 file changed

Lines changed: 18 additions & 12 deletions

File tree

‎compiler/ml/env.ml‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,15 +298,16 @@ module Id_tbl = struct
298298
(Opened {using; root; components; force; overrides = Tbl.empty; next});
299299
}
300300

301-
(* The binding named [name] in an open, with its position. *)
302-
let find_opened (type a) name (Opened o : a opened) : a * int =
301+
(* The path and binding named [name] in an open. The result is built here,
302+
so that a lookup allocates no other pair on the way. *)
303+
let find_opened (type a) name (Opened o : a opened) : Path.t * a =
303304
match Tbl.find_str name o.overrides with
304-
| found -> found
305+
| desc, pos -> (Pdot (o.root, name, pos), desc)
305306
| exception Not_found ->
306307
let desc, pos = Tbl.find_str name o.components in
307-
(o.force desc, pos)
308+
(Pdot (o.root, name, pos), o.force desc)
308309

309-
(* Every binding of an open, as [find_opened] gives it. *)
310+
(* Every binding of an open, as [find_opened] gives it, with its position. *)
310311
let fold_opened (type a) f (Opened o : a opened) acc =
311312
Tbl.fold
312313
(fun name (desc, pos) acc ->
@@ -328,10 +329,9 @@ module Id_tbl = struct
328329
(Pident id, desc)
329330
with Not_found as exn -> (
330331
match tbl.opened with
331-
| Some (Opened {using; root; next} as opened) -> (
332+
| Some (Opened {using; next} as opened) -> (
332333
try
333-
let descr, pos = find_opened name opened in
334-
let res = (Pdot (root, name, pos), descr) in
334+
let res = find_opened name opened in
335335
(if mark then
336336
match using with
337337
| None -> ()
@@ -351,8 +351,14 @@ module Id_tbl = struct
351351
{tbl with current = Ident.add id new_desc tbl.current}
352352
with Not_found -> (
353353
match tbl.opened with
354-
| Some (Opened o as opened) -> (
355-
match find_opened name opened with
354+
| Some (Opened o) -> (
355+
match
356+
match Tbl.find_str name o.overrides with
357+
| found -> found
358+
| exception Not_found ->
359+
let desc, pos = Tbl.find_str name o.components in
360+
(o.force desc, pos)
361+
with
356362
| desc, pos ->
357363
let overrides = Tbl.add name (f desc, pos) o.overrides in
358364
{tbl with opened = Some (Opened {o with overrides})}
@@ -368,9 +374,9 @@ module Id_tbl = struct
368374
@
369375
match tbl.opened with
370376
| None -> []
371-
| Some (Opened {root; next} as opened) -> (
377+
| Some (Opened {next} as opened) -> (
372378
match find_opened name opened with
373-
| desc, pos -> (Pdot (root, name, pos), desc) :: find_all name next
379+
| found -> found :: find_all name next
374380
| exception Not_found -> find_all name next)
375381

376382
let rec fold_name f tbl acc =

0 commit comments

Comments
 (0)