Skip to content

Commit 910c04f

Browse files
committed
Remember type path lookups within a compilation
Why: Typechecking a 552 module React app calls Env.find_type_full 1.76M times, and normalize_path 666k times from expand_abbrev. Most calls look up the same few paths against the same environment: `option` alone is 528k of them, followed by `string`, `array`, `Jsx.element`. Each lookup walks the open layers of env.types, and expanding a type that isn't an abbreviation also normalizes its path, which raises and catches Not_found to return the path unchanged. A direct-mapped table now remembers successful lookups and unchanged normalizations. An entry is only reused while the environment tables the lookup read are physically the same (env.types for a Pident, env.components for a Pdot, env.modules and env.components for a normalization), and only within the request that stored it. Changed normalizations are computed again, so paths in types are allocated as before. Paths with a functor application aren't cached, the memo is skipped while loading .cmi files is disabled, and it is emptied when a request ends and when a loaded module is replaced. The two tables start empty and grow from 64 to 4096 slots as entries are stored, and a replaced entry is updated in place, so a small compilation allocates little for them: fixed 4096 slot tables made each runtime file allocate about 3% more words than master. Now the file that grows most, Dom.res, allocates 0.24% more, and the runtime and Belt in total 0.06% less. On the corpus 94% of lookups and 96% of normalizations hit, and the output is byte identical to master. Review: Can an entry be stale? The tables an entry depends on are immutable, so the same physical table gives the same answer. New unit tests bind the same path differently in two environments within one request (recursive type placeholders, recursive module approximations, an alias) and fail if any of these checks is removed. Review: Can growing or reusing an entry return a wrong result? Growth moves each entry to the slot of its path in the larger table, and a lookup still checks the path, the epoch and the tables of each entry it finds. An entry is only referenced from its slot, so updating it in place changes no result a caller holds. A unit test looks up 3000 types across the growth and fails if a replaced entry keeps its old result. Signed-off-by: Jono Prest <jjprest@gmail.com>
1 parent a531dc3 commit 910c04f

12 files changed

Lines changed: 971 additions & 13 deletions

‎changes/type-path-memo.polish.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Build: remember type path lookups within a compilation, which avoids looking up the same types such as `option` and `string` hundreds of thousands of times per build.

‎compiler/ml/ctype.ml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,11 +1206,11 @@ let expand_abbrev_gen kind find_type_expansion env ty =
12061206
ty'
12071207
| None -> (
12081208
match find_type_expansion path env with
1209-
| exception Not_found ->
1209+
| exception Not_found -> (
12101210
(* another way to expand is to normalize the path itself *)
1211-
let path' = Env.normalize_path None env path in
1212-
if Path.same path path' then raise Cannot_expand
1213-
else newty2 level (Tconstr (path', args, abbrev))
1211+
match Env.normalize_type_path env path with
1212+
| None -> raise Cannot_expand
1213+
| Some path' -> newty2 level (Tconstr (path', args, abbrev)))
12141214
| params, body, lv ->
12151215
(* prerr_endline
12161216
("add a "^string_of_kind kind^" expansion for "^Path.name path);*)

‎compiler/ml/env.ml‎

Lines changed: 233 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -549,11 +549,156 @@ type pers_struct = {
549549
ps_filename: string;
550550
}
551551

552+
(* Request-local memo of type path lookups ([find_type_full] on regular
553+
paths) and of [normalize_path] calls that leave a type path unchanged.
554+
555+
The typechecker resolves the same type paths over and over (expanding
556+
[option], [Js.Null.t], ... in [expand_head], [update_level], [occur_univar],
557+
...), each time walking the [open] layers of [env.types] or the components
558+
of a persistent module. Both lookups only read immutable environment tables
559+
([env.types] for [Pident] type paths, [env.components] for [Pdot] ones,
560+
[env.modules] and [env.components] for normalization) plus request-wide
561+
state that only grows: persistent structures (replacing one starts a new
562+
memo epoch, see [add_persistent_structure]) and module components, which
563+
are forced at most once. An entry is therefore
564+
reused only when the tables it read are physically the same, and only
565+
successful results are stored. A repeated call has no side effect the memo
566+
would skip: the first, uncached call already loaded the [.cmi]s and forced
567+
the lazy components it needed. Paths that contain a functor application are
568+
never cached ([find_module_descr] applies the functor against [env]), and
569+
the memo is bypassed while [.cmi] loading is disabled ([without_cmis] may
570+
backtrack components). *)
571+
module Type_path_memo = struct
572+
(* Powers of two. A table starts empty, and only allocates its first array
573+
when it stores an entry, so that a process that doesn't typecheck (such as
574+
a parse with -bs-ast) allocates none. It grows when it has stored as many
575+
entries as it has slots since it last grew, which keeps a small
576+
compilation from allocating the full size. *)
577+
let initial_size = 64
578+
let max_size = 4096
579+
580+
let string_hash s =
581+
let n = String.length s in
582+
if n = 0 then 0
583+
else
584+
(n * 31)
585+
+ (Char.code (String.unsafe_get s 0) * 7)
586+
+ Char.code (String.unsafe_get s (n - 1))
587+
588+
exception Not_cacheable
589+
590+
(* Consistent with [equal]: it ignores [pos] and ident flags, which no lookup
591+
reads. *)
592+
let rec hash (path : Path.t) =
593+
match path with
594+
| Pident id -> (id.Ident.stamp * 65599) + string_hash id.name
595+
| Pdot (p, s, _pos) -> (hash p * 31) + string_hash s
596+
| Papply _ -> raise_notrace Not_cacheable
597+
598+
(* Paths that every lookup in this file resolves identically: identifiers are
599+
looked up by name and stamp, and components by name. *)
600+
let rec equal (p1 : Path.t) (p2 : Path.t) =
601+
p1 == p2
602+
||
603+
match (p1, p2) with
604+
| Pident id1, Pident id2 ->
605+
id1.Ident.stamp = id2.Ident.stamp && String.equal id1.name id2.name
606+
| Pdot (p1, s1, _), Pdot (p2, s2, _) -> String.equal s1 s2 && equal p1 p2
607+
| _ -> false
608+
609+
type ('types, 'modules, 'components, 'result) entry = {
610+
mutable epoch: unit ref;
611+
mutable path: Path.t;
612+
mutable types: 'types;
613+
mutable modules: 'modules;
614+
mutable components: 'components;
615+
mutable result: 'result;
616+
}
617+
618+
type ('types, 'modules, 'components, 'result) t = {
619+
mutable entries:
620+
('types, 'modules, 'components, 'result) entry option array;
621+
mutable stores: int; (** Since [entries] was allocated. *)
622+
}
623+
624+
let create () = {entries = [||]; stores = 0}
625+
626+
let clear t = Array.fill t.entries 0 (Array.length t.entries) None
627+
628+
(* The entry in the slot of a path whose [hash] is [h]. *)
629+
let find t h =
630+
let entries = t.entries in
631+
let n = Array.length entries in
632+
if n = 0 then None else Array.unsafe_get entries (h land (n - 1))
633+
634+
(* Stores an entry in the slot of a path whose [hash] is [h]. An entry is
635+
only referenced from its slot, so the one it replaces is updated in place
636+
instead of allocating another: a file that declares many types changes
637+
[env.types] often, and stores the paths it looks up again each time. *)
638+
let store t h ~epoch ~path ~types ~modules ~components ~result =
639+
let n = Array.length t.entries in
640+
if n = 0 then t.entries <- Array.make initial_size None
641+
else if t.stores >= n && n < max_size then (
642+
let old = t.entries in
643+
let entries = Array.make (n * 2) None in
644+
Array.iter
645+
(function
646+
| None -> ()
647+
| Some e as slot ->
648+
Array.unsafe_set entries (hash e.path land ((n * 2) - 1)) slot)
649+
old;
650+
t.entries <- entries;
651+
t.stores <- 0);
652+
t.stores <- t.stores + 1;
653+
let entries = t.entries in
654+
let i = h land (Array.length entries - 1) in
655+
match Array.unsafe_get entries i with
656+
| Some e ->
657+
e.epoch <- epoch;
658+
e.path <- path;
659+
e.types <- types;
660+
e.modules <- modules;
661+
e.components <- components;
662+
e.result <- result
663+
| None ->
664+
Array.unsafe_set entries i
665+
(Some {epoch; path; types; modules; components; result})
666+
end
667+
668+
(* The tables are allocated once per domain rather than per request: they can
669+
grow large, and requests are small. An entry is valid only in the epoch that
670+
stored it. Each request starts a fresh epoch (and a nested request restores
671+
the outer one when it returns), so entries never cross requests; clearing
672+
starts a new epoch too. *)
673+
let type_path_memo_key =
674+
Domain.DLS.new_key (fun () ->
675+
(Type_path_memo.create (), Type_path_memo.create ()))
676+
677+
let type_path_memo_epoch_key = Request_local.new_key (fun () -> ref (ref ()))
678+
let type_path_memo_epoch () = !(Domain.DLS.get type_path_memo_epoch_key)
679+
680+
let clear_type_path_memo_tables () =
681+
let found, unchanged = Domain.DLS.get type_path_memo_key in
682+
Type_path_memo.clear found;
683+
Type_path_memo.clear unchanged
684+
685+
let clear_type_path_memo () =
686+
Domain.DLS.get type_path_memo_epoch_key := ref ();
687+
clear_type_path_memo_tables ()
688+
552689
let persistent_structures_key =
553690
Request_local.new_key (fun () ->
554691
(Hashtbl.create 17 : (string, pers_struct option) Hashtbl.t))
555692
let persistent_structures () = Domain.DLS.get persistent_structures_key
556693

694+
(* [Hashtbl.add] shadows an earlier binding for [name], which the type path
695+
memo may have read (directly, or as a missing module), so replacing one
696+
starts a new memo epoch. *)
697+
let add_persistent_structure name ps =
698+
let tbl = persistent_structures () in
699+
if Hashtbl.mem tbl name then clear_type_path_memo ();
700+
Hashtbl.add tbl name ps
701+
557702
(* Consistency between persistent structures *)
558703

559704
let crc_units_key = Request_local.new_key Consistbl.create
@@ -590,7 +735,7 @@ let check_consistency ps =
590735

591736
let save_pers_struct crc ps =
592737
let modname = ps.ps_name in
593-
Hashtbl.add (persistent_structures ()) modname (Some ps);
738+
add_persistent_structure modname (Some ps);
594739
Consistbl.set (crc_units ()) modname crc ps.ps_filename;
595740
add_import modname
596741

@@ -635,7 +780,7 @@ let acknowledge_pers_struct check modname {Persistent_signature.filename; cmi} =
635780
if ps.ps_name <> modname then
636781
error (Illegal_renaming (modname, ps.ps_name, filename));
637782
if check then check_consistency ps;
638-
Hashtbl.add (persistent_structures ()) modname (Some ps);
783+
add_persistent_structure modname (Some ps);
639784
ps
640785

641786
let read_pers_struct check modname filename =
@@ -656,7 +801,7 @@ let find_pers_struct check name =
656801
match !Persistent_signature.load ~unit_name:name with
657802
| Some ps -> ps
658803
| None ->
659-
Hashtbl.add (persistent_structures ()) name None;
804+
add_persistent_structure name None;
660805
raise Not_found
661806
in
662807
add_import name;
@@ -701,6 +846,7 @@ let check_pers_struct name =
701846

702847
let reset_cache () =
703848
current_unit () := "";
849+
clear_type_path_memo ();
704850
Hashtbl.clear (persistent_structures ());
705851
clear_imports ();
706852
Hashtbl.clear (value_declarations ());
@@ -717,13 +863,16 @@ let reset_cache_toplevel () =
717863
(persistent_structures ()) []
718864
in
719865
List.iter (Hashtbl.remove (persistent_structures ())) l;
866+
clear_type_path_memo ();
720867
Hashtbl.clear (value_declarations ());
721868
Hashtbl.clear (type_declarations ());
722869
Hashtbl.clear (module_declarations ());
723870
Hashtbl.clear (used_constructors ());
724871
Hashtbl.clear (prefixed_sg ())
725872

726-
let set_unit_name name = current_unit () := name
873+
let set_unit_name name =
874+
clear_type_path_memo ();
875+
current_unit () := name
727876

728877
let get_unit_name () = !(current_unit ())
729878

@@ -770,11 +919,36 @@ let type_of_cstr path = function
770919
(d, ([], List.map snd (Datarepr.labels_of_type path d)))
771920
| _ -> assert false
772921

922+
(* [find_type_full p env] for a regular type path, memoized as described at
923+
[Type_path_memo]. *)
924+
let find_regular_type_full p env =
925+
match !(can_load_cmis ()) with
926+
| Cannot_load_cmis _ -> find_type_full p env
927+
| Can_load_cmis -> (
928+
match Type_path_memo.hash p with
929+
| exception Type_path_memo.Not_cacheable -> find_type_full p env
930+
| h -> (
931+
let memo, _ = Domain.DLS.get type_path_memo_key in
932+
let epoch = type_path_memo_epoch () in
933+
match Type_path_memo.find memo h with
934+
| Some e
935+
when e.epoch == epoch
936+
&& (match p with
937+
| Pident _ -> e.types == env.types
938+
| Pdot _ | Papply _ -> e.components == env.components)
939+
&& Type_path_memo.equal e.path p ->
940+
e.result
941+
| _ ->
942+
let result = find_type_full p env in
943+
Type_path_memo.store memo h ~epoch ~path:p ~types:env.types ~modules:()
944+
~components:env.components ~result;
945+
result))
946+
773947
let find_type_full path env =
774948
match Path.constructor_typath path with
775949
| Regular p -> (
776950
try (Path_map.find p env.local_constraints, ([], []))
777-
with Not_found -> find_type_full p env)
951+
with Not_found -> find_regular_type_full p env)
778952
| Cstr (ty_path, s) ->
779953
let _, (cstrs, _) =
780954
try find_type_full ty_path env with Not_found -> assert false
@@ -881,6 +1055,41 @@ let normalize_path oloc env path =
8811055
| Some loc ->
8821056
raise (Error (Missing_module (loc, path, normalize_path true env path))))
8831057

1058+
(* [Some (normalize_path None env path)] when normalization changes [path]
1059+
([Path.same] is false), [None] when it leaves it unchanged. The unchanged
1060+
outcome is memoized as described at [Type_path_memo]; a changed path is
1061+
always computed afresh, so callers get a new path value as before. *)
1062+
(* Not a closure in [normalize_type_path], which would be allocated on every
1063+
call, hits included. *)
1064+
let normalize_type_path_uncached env path =
1065+
let path' = normalize_path None env path in
1066+
if Path.same path path' then None else Some path'
1067+
1068+
let normalize_type_path env path =
1069+
match !(can_load_cmis ()) with
1070+
| Cannot_load_cmis _ -> normalize_type_path_uncached env path
1071+
| Can_load_cmis -> (
1072+
match Type_path_memo.hash path with
1073+
| exception Type_path_memo.Not_cacheable ->
1074+
normalize_type_path_uncached env path
1075+
| h -> (
1076+
let _, memo = Domain.DLS.get type_path_memo_key in
1077+
let epoch = type_path_memo_epoch () in
1078+
match Type_path_memo.find memo h with
1079+
| Some e
1080+
when e.epoch == epoch && e.modules == env.modules
1081+
&& e.components == env.components
1082+
&& Type_path_memo.equal e.path path ->
1083+
None
1084+
| _ ->
1085+
let result = normalize_type_path_uncached env path in
1086+
(match result with
1087+
| None ->
1088+
Type_path_memo.store memo h ~epoch ~path ~types:()
1089+
~modules:env.modules ~components:env.components ~result:()
1090+
| Some _ -> ());
1091+
result))
1092+
8841093
let normalize_path_prefix oloc env path =
8851094
match path with
8861095
| Pdot (p, s, pos) -> Pdot (normalize_path oloc env p, s, pos)
@@ -2074,7 +2283,25 @@ let reset_initial_for_request () =
20742283
let with_fresh_initial action =
20752284
let previous = initial_safe_string () in
20762285
Fun.protect action ~finally:(fun () ->
2077-
Domain.DLS.set initial_safe_string_key previous)
2286+
Domain.DLS.set initial_safe_string_key previous;
2287+
(* The per-domain memo tables would otherwise keep the finished
2288+
request's environments alive until their slots are overwritten. Their
2289+
entries are already invalid outside the request's epoch. *)
2290+
clear_type_path_memo_tables ())
2291+
2292+
module For_test = struct
2293+
let type_path_memo_entries () =
2294+
let found, unchanged = Domain.DLS.get type_path_memo_key in
2295+
let count (t : _ Type_path_memo.t) =
2296+
Array.fold_left
2297+
(fun n e ->
2298+
match e with
2299+
| None -> n
2300+
| Some _ -> n + 1)
2301+
0 t.entries
2302+
in
2303+
count found + count unchanged
2304+
end
20782305

20792306
let last_env_key = Request_local.new_key (fun () -> ref empty)
20802307
let last_env () = Domain.DLS.get last_env_key

‎compiler/ml/env.mli‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ val empty : t
2626
val initial_safe_string : unit -> t
2727
val reset_initial_for_request : unit -> unit
2828
val with_fresh_initial : (unit -> 'a) -> 'a
29+
(* Runs a request with its own initial environment; on return, also empties
30+
this domain's type path memo. *)
2931

3032
type type_descriptions = constructor_description list * label_description list
3133

@@ -65,6 +67,10 @@ val normalize_path : Location.t option -> t -> Path.t -> Path.t
6567
If the option is None, allow returning dangling paths.
6668
Otherwise raise a Missing_module error, and may add forgotten
6769
head as required global. *)
70+
val normalize_type_path : t -> Path.t -> Path.t option
71+
(* [normalize_type_path env path] is [Some (normalize_path None env path)] when
72+
that differs from [path] according to [Path.same], and [None] otherwise. *)
73+
6874
val normalize_path_prefix : Location.t option -> t -> Path.t -> Path.t
6975
(* Only normalize the prefix part of the path *)
7076

@@ -285,3 +291,8 @@ module Persistent_signature : sig
285291
the .cmi file in the load path. This function can be overridden to load
286292
it from memory, for instance to build a self-contained toplevel. *)
287293
end
294+
295+
module For_test : sig
296+
val type_path_memo_entries : unit -> int
297+
(** The type path memo entries held on this domain. *)
298+
end

‎compiler/ml/typecore.ml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -814,9 +814,10 @@ let rec expand_path env p =
814814
match repr ty with
815815
| {desc = Tconstr (p, _, _)} -> expand_path env p
816816
| _ -> p (* PR#6394: recursive module may introduce incoherent manifest *))
817-
| _ ->
818-
let p' = Env.normalize_path None env p in
819-
if Path.same p p' then p else expand_path env p'
817+
| _ -> (
818+
match Env.normalize_type_path env p with
819+
| None -> p
820+
| Some p' -> expand_path env p')
820821

821822
let compare_type_path env tpath1 tpath2 =
822823
Path.same (expand_path env tpath1) (expand_path env tpath2)

0 commit comments

Comments
 (0)