Skip to content

Commit 1eb91bb

Browse files
cknittclaude
andauthored
Remove the hashing fast paths from pattern matching (#8900)
#8868 made exhaustiveness checking and match compilation find constants by hashing, because an expanded char range turned one pattern into thousands of distinct constants. Char ranges are now one pattern each, so the hash tables in `Parmatch.filter_all`, `Parmatch.get_mins_pats` and `Matching.divide`, and the `const_key` that kept their keys consistent with char/int equality, are removed. A dead `char` check in comparison specialization goes too: `char` expands to `int` before it. Review: How much slower do large switches compile? A switch with 1,000 int or string cases compiles 0.02-0.03 s slower; one with 10,000 int cases takes 6.3 s instead of 3.2 s. Polymorphic variants are unaffected: 10,000 cases take about 35 s either way, as other quadratic steps dominate. Review: Does the output change? No. All committed snapshots are byte-identical, and so is the JS of 1,500 fuzzer programs and the benchmark switches. Area: typechecker lambda Output: unchanged Formatting: unchanged Formats: none PPX: unchanged Editor: unchanged Tests: none, because the output is unchanged and existing tests cover the behaviour Measured: best of 3 bsc wall time, with -> without the fast paths: 1,000 int cases 0.07 -> 0.09 s; 3,000 int 0.34 -> 0.63 s; 10,000 int 3.23 -> 6.27 s; 1,000 string 0.04 -> 0.07 s; 3,000 string 0.17 -> 0.52 s; 10,000 string 1.55 -> 5.38 s; 1,000 polymorphic variant 0.23 -> 0.25 s; 10,000 polymorphic variant 35.47 -> 36.45 s Claude-Session: https://claude.ai/code/session_01JALXUyHhkDqzcB3WuW9Svo Signed-off-by: Christoph Knittel <ck@cca.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 407c384 commit 1eb91bb

5 files changed

Lines changed: 21 additions & 111 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Remove the hashing fast paths for switches with many distinct constants, which char ranges no longer need.

‎compiler/ml/matching.ml‎

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ let rec what_is_cases cases =
631631
| _ -> assert false
632632

633633
(* A few operations on default environments *)
634-
let as_matrix cases = get_mins_pats (List.map (fun (ps, _) -> ps) cases)
634+
let as_matrix cases = get_mins le_pats (List.map (fun (ps, _) -> ps) cases)
635635

636636
let cons_default matrix raise_num default =
637637
match matrix with
@@ -1168,30 +1168,13 @@ let add make_matching_fun division eq_key key patl_action args =
11681168
cell.pm.cases <- [patl_action];
11691169
(key, cell) :: division
11701170

1171-
(* Cells whose key has a [hash_key] are also found by hashing on it, which
1172-
must agree with [eq_key]. This keeps a column of many distinct keys
1173-
linear. *)
1174-
let divide ?(hash_key = fun _ -> None) make eq_key get_key get_args ctx pm =
1175-
let cells = lazy (Hashtbl.create 16) in
1171+
let divide make eq_key get_key get_args ctx pm =
11761172
let rec divide_rec = function
1177-
| (p :: patl, action) :: rem -> (
1173+
| (p :: patl, action) :: rem ->
11781174
let this_match = divide_rec rem in
1179-
let key = get_key p in
1180-
let patl_action = (get_args p patl, action) in
1181-
match hash_key key with
1182-
| None ->
1183-
add (make p pm.default ctx) this_match eq_key key patl_action pm.args
1184-
| Some hkey -> (
1185-
let cells = Lazy.force cells in
1186-
match Hashtbl.find_opt cells hkey with
1187-
| Some cell ->
1188-
cell.pm.cases <- patl_action :: cell.pm.cases;
1189-
this_match
1190-
| None ->
1191-
let cell = make p pm.default ctx pm.args in
1192-
cell.pm.cases <- [patl_action];
1193-
Hashtbl.add cells hkey cell;
1194-
(key, cell) :: this_match))
1175+
add (make p pm.default ctx) this_match eq_key (get_key p)
1176+
(get_args p patl, action)
1177+
pm.args
11951178
| _ -> []
11961179
in
11971180

@@ -1342,12 +1325,7 @@ let divide_constant ctx m =
13421325
match column_int_ranges m with
13431326
| Some ranges -> divide_constant_ranges ranges ctx m
13441327
| None ->
1345-
divide
1346-
~hash_key:(function
1347-
| ((Asttypes.Const_int _ | Const_char _ | Const_string _) as c), _ ->
1348-
Some (Parmatch.const_key c)
1349-
| (Const_float _ | Const_bigint _), _ -> None)
1350-
make_constant_matching
1328+
divide make_constant_matching
13511329
(fun (c, _) (d, _) -> const_compare c d = 0)
13521330
(get_key_constant "divide")
13531331
get_args_constant ctx m

‎compiler/ml/parmatch.ml‎

Lines changed: 13 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -325,12 +325,6 @@ let const_compare x y =
325325
compare i1 i2
326326
| _, _ -> compare x y
327327

328-
(* [char] is an alias of [int], so a char constant and an int constant with the
329-
same value match the same values. Hash tables of constants use this key. *)
330-
let const_key = function
331-
| Const_char i -> Const_int i
332-
| c -> c
333-
334328
let records_args l1 l2 =
335329
(* Invariant: fields are already sorted by Typecore.type_record_elem_list *)
336330
let rec combine r1 r2 l1 l2 =
@@ -804,22 +798,14 @@ let filter_extra pss =
804798

805799
let filter_all pat0 pss =
806800
(* The groups, most recently created first. Each row goes to the group
807-
whose head [simple_match]es it; there is at most one such group, since a
808-
group is only created when no other matches. *)
801+
whose head matches it; there is at most one such group, since a group is
802+
only created when no other matches. *)
809803
let groups =
810804
ref
811805
(match pat0.pat_desc with
812806
| Tpat_record _ | Tpat_tuple _ -> [(pat0, ref [])]
813807
| _ -> [])
814808
in
815-
(* For int, char and string constants and for polymorphic variants,
816-
[simple_match] is equality, so their groups are also found by hashing.
817-
This keeps a column of many distinct heads, e.g. an expanded character
818-
range, linear instead of quadratic. The tables are only created when needed,
819-
as most calls are on small matrices. *)
820-
let const_groups = lazy (Hashtbl.create 16) in
821-
let interval_groups = lazy (Hashtbl.create 16) in
822-
let variant_groups = lazy (Hashtbl.create 16) in
823809
let add_row (q0, rows) q qs =
824810
rows := (simple_match_args q0 q @ qs) :: !rows
825811
in
@@ -830,24 +816,17 @@ let filter_all pat0 pss =
830816
groups := group :: !groups;
831817
group
832818
in
833-
let find_or_add tbl key q qs =
834-
let tbl = Lazy.force tbl in
835-
match Hashtbl.find_opt tbl key with
836-
| Some group -> add_row group q qs
837-
| None -> Hashtbl.add tbl key (new_group q qs)
819+
(* Int and char ranges are grouped by equal bounds, not by containment as
820+
[simple_match] does, so that [build_other] sees every range. *)
821+
let same_head q0 q =
822+
match (int_range q0, int_range q) with
823+
| Some r0, Some r -> r0 = r
824+
| _ -> simple_match q0 q
838825
in
839826
let insert q qs =
840-
match q.pat_desc with
841-
| Tpat_constant ((Const_int _ | Const_char _ | Const_string _) as c) ->
842-
find_or_add const_groups (const_key c) q qs
843-
| Tpat_interval (c1, c2) ->
844-
(* Grouped by equal bounds; [build_other] then sees every interval. *)
845-
find_or_add interval_groups (const_key c1, const_key c2) q qs
846-
| Tpat_variant (l, _, _) -> find_or_add variant_groups l q qs
847-
| _ -> (
848-
match List.find_opt (fun (q0, _) -> simple_match q0 q) !groups with
849-
| Some group -> add_row group q qs
850-
| None -> ignore (new_group q qs))
827+
match List.find_opt (fun (q0, _) -> same_head q0 q) !groups with
828+
| Some group -> add_row group q qs
829+
| None -> ignore (new_group q qs)
851830
in
852831

853832
let rec filter_rec = function
@@ -1842,49 +1821,6 @@ let get_mins le ps =
18421821
in
18431822
select_rec [] (select_rec [] ps)
18441823

1845-
(* [get_mins le_pats pss], but faster for many rows starting with distinct
1846-
constants, e.g. an expanded character range. A row starting with an int,
1847-
char or string constant can only be covered by rows starting with the same
1848-
constant or with another kind of pattern, so only those are compared. *)
1849-
let get_mins_pats pss =
1850-
let key = function
1851-
| {
1852-
pat_desc =
1853-
Tpat_constant ((Const_int _ | Const_char _ | Const_string _) as c);
1854-
}
1855-
:: _ ->
1856-
Some (const_key c)
1857-
| _ -> None
1858-
in
1859-
(* [select_rec [] pss] of [get_mins le_pats]: drops each row covered by a
1860-
later row, and reverses the order. *)
1861-
let select pss =
1862-
let later = ref [] in
1863-
let later_by_key = lazy (Hashtbl.create 16) in
1864-
let later_unkeyed = ref [] in
1865-
let kept =
1866-
List.fold_left
1867-
(fun kept ps ->
1868-
let below qs = le_pats qs ps in
1869-
let key = key ps in
1870-
let dominated =
1871-
match key with
1872-
| None -> List.exists below !later
1873-
| Some c ->
1874-
List.exists below (Hashtbl.find_all (Lazy.force later_by_key) c)
1875-
|| List.exists below !later_unkeyed
1876-
in
1877-
later := ps :: !later;
1878-
(match key with
1879-
| None -> later_unkeyed := ps :: !later_unkeyed
1880-
| Some c -> Hashtbl.add (Lazy.force later_by_key) c ps);
1881-
if dominated then kept else ps :: kept)
1882-
[] (List.rev pss)
1883-
in
1884-
List.rev kept
1885-
in
1886-
select (select pss)
1887-
18881824
(*
18891825
lub p q is a pattern that matches all values matched by p and q
18901826
may raise Empty, when p and q are not compatible
@@ -2340,7 +2276,7 @@ let check_unused pred casel =
23402276
| {c_lhs = q; c_guard} :: rem ->
23412277
let qs = [q] in
23422278
(try
2343-
let pss = get_mins_pats (Ext_list.filter pref (compats qs)) in
2279+
let pss = get_mins le_pats (Ext_list.filter pref (compats qs)) in
23442280
(* First look for redundant or partially redundant patterns *)
23452281
let r = every_satisfiables (make_rows pss) (make_row qs) in
23462282
(* Do not warn for unused [pat -> .] *)
@@ -2419,7 +2355,7 @@ let irrefutable pat = le_pat (Dict_pattern.lowering [pat] pat) omega
24192355

24202356
let check_partial_param do_check_partial do_check_fragile loc casel =
24212357
let pss = initial_matrix casel in
2422-
let pss = get_mins_pats pss in
2358+
let pss = get_mins le_pats pss in
24232359
let total = do_check_partial loc casel pss in
24242360
if total = Total && Warnings.is_active (Warnings.Fragile_match "") then
24252361
do_check_fragile loc casel pss;

‎compiler/ml/parmatch.mli‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ val all_record_args :
3434
(Longident.t loc * label_description * pattern * bool) list
3535
val const_compare : constant -> constant -> int
3636

37-
(* A hash key agreeing with [const_compare]: char constants map to ints. *)
38-
val const_key : constant -> constant
39-
4037
(* The bounds of an int or char constant or interval pattern *)
4138
val int_range : pattern -> (int * int) option
4239

@@ -59,7 +56,6 @@ val lub : pattern -> pattern -> pattern
5956
val lubs : pattern list -> pattern list -> pattern list
6057

6158
val get_mins : ('a -> 'a -> bool) -> 'a list -> 'a list
62-
val get_mins_pats : pattern list list -> pattern list list
6359

6460
(* Those two functions recombine one pattern and its arguments:
6561
For instance:

‎compiler/ml/translcore.ml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,6 @@ let specialize_comparison
401401
match () with
402402
| ()
403403
when is_base_type env ty Predef.path_int
404-
|| is_base_type env ty Predef.path_char
405404
|| maybe_pointer_type env ty = Immediate ->
406405
intcomp
407406
| () when is_base_type env ty Predef.path_float -> floatcomp

0 commit comments

Comments
 (0)