Skip to content

Commit 5d69ca2

Browse files
cknittclaude
andcommitted
Check large switches for unused cases without comparing every pair
Why: The unused-case check compared each case with every earlier one and retyped part of each case, and its usefulness checks grouped rows by scanning all groups, so a switch with n constant or polymorphic variant cases cost O(n²) in several places: 6,000 int cases took 1.65 s and allocated 430M words, 1,600 polymorphic variant cases 1.31 s and 242M. They now take 0.56 s with 74M words, and 0.74 s with 128M. Rows are indexed by the constant or tag at their head, which a row can only share with rows of the same head or of a head without one, such as a wildcard or a range: the earlier rows compatible with a case, the groups of `filter_all` and the most general rows of `get_mins_pats` are found through that index. A case is retyped to find GADT cases that can't match only when its pattern has a GADT constructor, as no other pattern can fail to type. Warnings are the same for the test suites and 2,600 generated switches. Review: What remains superlinear? The match compiler (`Matching.divide` and the switch clustering) for constant switches: 6,000 int cases allocate 69M words with the warnings off, 3.4× the words for 3,000. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gK1dcjbe97yY1wRMgs Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent bf35764 commit 5d69ca2

6 files changed

Lines changed: 260 additions & 13 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Speed up the unused-case check of switches with many constant or polymorphic variant cases.

‎compiler/ml/matching.ml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ let rec what_is_cases cases =
640640
| _ -> assert false
641641

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

645645
let cons_default matrix raise_num default =
646646
match matrix with

‎compiler/ml/parmatch.ml‎

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

328+
(* The head of a pattern when it is a constant or a polymorphic variant tag.
329+
Two patterns with such heads are compatible, and one includes the other,
330+
only when the heads are equal. *)
331+
type head_key = Head_constant of constant | Head_variant of label
332+
333+
let rec head_key p =
334+
match p.pat_desc with
335+
| Tpat_alias (p, _, _) -> head_key p
336+
| Tpat_constant (Const_char i) -> Some (Head_constant (Const_int i))
337+
| Tpat_constant c -> Some (Head_constant c)
338+
| Tpat_variant (l, _, _) -> Some (Head_variant l)
339+
| _ -> None
340+
341+
module Head_map = Map.Make (struct
342+
type t = head_key
343+
let compare k1 k2 =
344+
match (k1, k2) with
345+
| Head_constant c1, Head_constant c2 -> const_compare c1 c2
346+
| Head_variant l1, Head_variant l2 -> String.compare l1 l2
347+
| Head_constant _, Head_variant _ -> -1
348+
| Head_variant _, Head_constant _ -> 1
349+
end)
350+
328351
let records_args l1 l2 =
329352
(* Invariant: fields are already sorted by Typecore.type_record_elem_list *)
330353
let rec combine r1 r2 l1 l2 =
@@ -809,11 +832,20 @@ let filter_all pat0 pss =
809832
let add_row (q0, rows) q qs =
810833
rows := (simple_match_args q0 q @ qs) :: !rows
811834
in
835+
(* The groups again, those with a [head_key] by key: a row with a key
836+
can only belong to the group of the same key, or to a group without one
837+
(the range [5 .. 5] has the head of the constant [5]). Looking through
838+
all groups for each row is quadratic in a switch on many constants. *)
839+
let keyed = ref Head_map.empty in
840+
let unkeyed = ref !groups in
812841
let new_group q qs =
813842
let q0 = normalize_pat q in
814843
let group = (q0, ref []) in
815844
add_row group q qs;
816845
groups := group :: !groups;
846+
(match head_key q0 with
847+
| Some key -> keyed := Head_map.add key group !keyed
848+
| None -> unkeyed := group :: !unkeyed);
817849
group
818850
in
819851
(* Int and char ranges are grouped by equal bounds, not by containment as
@@ -824,7 +856,15 @@ let filter_all pat0 pss =
824856
| _ -> simple_match q0 q
825857
in
826858
let insert q qs =
827-
match List.find_opt (fun (q0, _) -> same_head q0 q) !groups with
859+
let group =
860+
match head_key q with
861+
| Some key -> (
862+
match Head_map.find_opt key !keyed with
863+
| Some group -> Some group
864+
| None -> List.find_opt (fun (q0, _) -> same_head q0 q) !unkeyed)
865+
| None -> List.find_opt (fun (q0, _) -> same_head q0 q) !groups
866+
in
867+
match group with
828868
| Some group -> add_row group q qs
829869
| None -> ignore (new_group q qs)
830870
in
@@ -1782,6 +1822,47 @@ let get_mins le ps =
17821822
in
17831823
select_rec [] (select_rec [] ps)
17841824

1825+
let row_head_key = function
1826+
| p :: _ -> head_key p
1827+
| [] -> None
1828+
1829+
(* [get_mins le_pats pss], comparing a row with a [head_key] only with the
1830+
rows whose first pattern has the same key or none: comparing all pairs is
1831+
quadratic for a switch on many constants or tags. *)
1832+
let get_mins_pats pss =
1833+
(* The rows that no later row includes, in order. *)
1834+
let select pss =
1835+
let rec go keyed unkeyed kept = function
1836+
| [] -> kept
1837+
| ps :: rest ->
1838+
let includes ps0 = le_pats ps0 ps in
1839+
let key = row_head_key ps in
1840+
let included =
1841+
List.exists includes unkeyed
1842+
||
1843+
match key with
1844+
| Some key -> (
1845+
match Head_map.find_opt key keyed with
1846+
| Some rows -> List.exists includes rows
1847+
| None -> false)
1848+
| None ->
1849+
Head_map.exists (fun _ rows -> List.exists includes rows) keyed
1850+
in
1851+
let keyed, unkeyed =
1852+
match key with
1853+
| Some key ->
1854+
( Head_map.update key
1855+
(fun rows -> Some (ps :: Option.value rows ~default:[]))
1856+
keyed,
1857+
unkeyed )
1858+
| None -> (keyed, ps :: unkeyed)
1859+
in
1860+
go keyed unkeyed (if included then kept else ps :: kept) rest
1861+
in
1862+
go Head_map.empty [] [] (List.rev pss)
1863+
in
1864+
List.rev (select (List.rev (select pss)))
1865+
17851866
(*
17861867
lub p q is a pattern that matches all values matched by p and q
17871868
may raise Empty, when p and q are not compatible
@@ -2424,15 +2505,54 @@ let check_unused pred casel =
24242505
if unused_match || Warnings.is_active (Warnings.Unused_pat "") then
24252506
let casel = lower_cases casel in
24262507
let dead_cases, already_reported = check_shadowed_payloads casel in
2427-
let rec do_rec pref = function
2508+
(* The earlier rows, most recent first, numbered from 0, and grouped by
2509+
[head_key]: a case with a key is only compatible with the rows of the
2510+
same key and the rows without one. *)
2511+
let rec merge_rows acc rows1 rows2 =
2512+
match (rows1, rows2) with
2513+
| [], rows | rows, [] -> List.rev_append acc (List.map snd rows)
2514+
| (i1, ps1) :: rest1, (i2, _) :: _ when i1 > i2 ->
2515+
merge_rows (ps1 :: acc) rest1 rows2
2516+
| _, (_, ps2) :: rest2 -> merge_rows (ps2 :: acc) rows1 rest2
2517+
in
2518+
let compatible_rows qs (pref, keyed, unkeyed) =
2519+
match row_head_key qs with
2520+
| None -> Ext_list.filter (List.map snd pref) (compats qs)
2521+
| Some key ->
2522+
let keyed =
2523+
match Head_map.find_opt key keyed with
2524+
| Some rows -> rows
2525+
| None -> []
2526+
in
2527+
Ext_list.filter (merge_rows [] keyed unkeyed) (compats qs)
2528+
in
2529+
let add_row qs (pref, keyed, unkeyed) =
2530+
let index =
2531+
match pref with
2532+
| (index, _) :: _ -> index + 1
2533+
| [] -> 0
2534+
in
2535+
let row = (index, qs) in
2536+
let pref = row :: pref in
2537+
match row_head_key qs with
2538+
| Some key ->
2539+
( pref,
2540+
Head_map.update key
2541+
(fun rows -> Some (row :: Option.value rows ~default:[]))
2542+
keyed,
2543+
unkeyed )
2544+
| None -> (pref, keyed, row :: unkeyed)
2545+
in
2546+
let rec do_rec rows = function
24282547
| [] -> ()
24292548
| case :: rem when List.memq case dead_cases ->
24302549
(* A case that matches nothing cannot make later cases unused *)
2431-
do_rec pref rem
2550+
do_rec rows rem
24322551
| {c_lhs = q; c_guard} :: rem ->
24332552
let qs = [q] in
2553+
let pref, _, _ = rows in
24342554
(try
2435-
let pss = get_mins le_pats (Ext_list.filter pref (compats qs)) in
2555+
let pss = get_mins_pats (compatible_rows qs rows) in
24362556
(* First look for redundant or partially redundant patterns *)
24372557
let r = every_satisfiables (make_rows pss) (make_row qs) in
24382558
(* Do not warn for unused [pat -> .] *)
@@ -2472,11 +2592,22 @@ let check_unused pred casel =
24722592
(*Format.eprintf "%a@." pretty_val u;*)
24732593
let pattern, constrs, labels = Conv.conv u in
24742594
let pattern = {pattern with Parsetree.ppat_loc = q.pat_loc} in
2475-
match pred constrs labels pattern with
2476-
| None ->
2477-
Location.prerr_warning q.pat_loc Warnings.Unreachable_case;
2478-
Used
2479-
| _ -> r
2595+
(* Only a GADT constructor can make a pattern built from
2596+
the case's own subpatterns fail to type. Typing it
2597+
again costs as much as the type of the scrutinee, for
2598+
every case. *)
2599+
let has_gadt_constructor =
2600+
Hashtbl.fold
2601+
(fun _ cstr found -> found || cstr.cstr_generalized)
2602+
constrs false
2603+
in
2604+
if not has_gadt_constructor then r
2605+
else
2606+
match pred constrs labels pattern with
2607+
| None ->
2608+
Location.prerr_warning q.pat_loc Warnings.Unreachable_case;
2609+
Used
2610+
| _ -> r
24802611
in
24812612
match r with
24822613
| Unused ->
@@ -2492,10 +2623,11 @@ let check_unused pred casel =
24922623
| Used -> ()
24932624
with Empty | Not_found | NoGuard -> assert false);
24942625

2495-
if c_guard <> None then do_rec pref rem else do_rec ([q] :: pref) rem
2626+
if c_guard <> None then do_rec rows rem
2627+
else do_rec (add_row qs rows) rem
24962628
in
24972629

2498-
if unused_match then do_rec [] casel
2630+
if unused_match then do_rec ([], Head_map.empty, []) casel
24992631

25002632
(*********************************)
25012633
(* Exported irrefutability tests *)
@@ -2514,7 +2646,7 @@ let irrefutable pat = le_pat (Dict_pattern.lowering [pat] pat) omega
25142646

25152647
let check_partial_param do_check_partial do_check_fragile loc casel =
25162648
let pss = initial_matrix casel in
2517-
let pss = get_mins le_pats pss in
2649+
let pss = get_mins_pats pss in
25182650
let total = do_check_partial loc casel pss in
25192651
if total = Total && Warnings.is_active (Warnings.Fragile_match "") then
25202652
do_check_fragile loc casel pss;

‎compiler/ml/parmatch.mli‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ val lubs : pattern list -> pattern list -> pattern list
5757

5858
val get_mins : ('a -> 'a -> bool) -> 'a list -> 'a list
5959

60+
(* [get_mins le_pats], without comparing rows whose first patterns are
61+
different constants or polymorphic variant tags. *)
62+
val get_mins_pats : pattern list list -> pattern list list
63+
6064
(* Those two functions recombine one pattern and its arguments:
6165
For instance:
6266
(_,_)::p1::p2::rem -> (p1, p2)::rem
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
2+
Warning number 11
3+
/.../fixtures/warning_11_unused_constants.res:4:5
4+
5+
2 ┆ switch x {
6+
3 ┆ | 5 .. 5 => 0
7+
4 ┆ | 5 => 1
8+
5 ┆ | 1 .. 3 => 2
9+
6 ┆ | 2 => 3
10+
11+
this match case is unused.
12+
13+
14+
Warning number 11
15+
/.../fixtures/warning_11_unused_constants.res:6:5
16+
17+
4 ┆ | 5 => 1
18+
5 ┆ | 1 .. 3 => 2
19+
6 ┆ | 2 => 3
20+
7 ┆ | 4 | 7 => 4
21+
8 ┆ | 7 => 5
22+
23+
this match case is unused.
24+
25+
26+
Warning number 11
27+
/.../fixtures/warning_11_unused_constants.res:8:5
28+
29+
6 ┆ | 2 => 3
30+
7 ┆ | 4 | 7 => 4
31+
8 ┆ | 7 => 5
32+
9 ┆ | _ => 6
33+
10 ┆ }
34+
35+
this match case is unused.
36+
37+
38+
Warning number 12
39+
/.../fixtures/warning_11_unused_constants.res:15:11-13
40+
41+
13 ┆ switch x {
42+
14 ┆ | "a" => 0
43+
15 ┆ | "b" | "a" => 1
44+
16 ┆ | "b" => 2
45+
17 ┆ | _ => 3
46+
47+
this sub-pattern is unused.
48+
49+
50+
Warning number 11
51+
/.../fixtures/warning_11_unused_constants.res:16:5-7
52+
53+
14 ┆ | "a" => 0
54+
15 ┆ | "b" | "a" => 1
55+
16 ┆ | "b" => 2
56+
17 ┆ | _ => 3
57+
18 ┆ }
58+
59+
this match case is unused.
60+
61+
62+
Warning number 11
63+
/.../fixtures/warning_11_unused_constants.res:25:5-9
64+
65+
23 ┆ | #B(1) => 1
66+
24 ┆ | #B(_) => 2
67+
25 ┆ | #B(1) => 3
68+
26 ┆ | #A | #C => 4
69+
27 ┆ }
70+
71+
this match case is unused.
72+
73+
74+
Warning number 12
75+
/.../fixtures/warning_11_unused_constants.res:26:5-6
76+
77+
24 ┆ | #B(_) => 2
78+
25 ┆ | #B(1) => 3
79+
26 ┆ | #A | #C => 4
80+
27 ┆ }
81+
28 ┆
82+
83+
this sub-pattern is unused.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
let int = x =>
2+
switch x {
3+
| 5 .. 5 => 0
4+
| 5 => 1
5+
| 1 .. 3 => 2
6+
| 2 => 3
7+
| 4 | 7 => 4
8+
| 7 => 5
9+
| _ => 6
10+
}
11+
12+
let string = x =>
13+
switch x {
14+
| "a" => 0
15+
| "b" | "a" => 1
16+
| "b" => 2
17+
| _ => 3
18+
}
19+
20+
let variant = (x: [#A | #B(int) | #C]) =>
21+
switch x {
22+
| #A => 0
23+
| #B(1) => 1
24+
| #B(_) => 2
25+
| #B(1) => 3
26+
| #A | #C => 4
27+
}

0 commit comments

Comments
 (0)