Skip to content

Commit c25272c

Browse files
committed
Make issues_by_file reactive
- issues_by_file is now a reactive flatMap from dead_decls_by_file - Per-file issue generation with shouldReport callback (no mutations) - Module issues generated separately from dead_modules + modules_with_reported_values - Correctly handles flatMap not tracking reads from other collections Timing improvement on benchmark (cache hit): - iter_issues: ~1.5ms (down from ~32ms) - iter_modules: ~5-7ms (new, iterates dead_modules) - Total dead_code: ~12-14ms (down from ~38-40ms) All 380/19000 issues still match between reactive and non-reactive modes.
1 parent 5ef231e commit c25272c

File tree

1 file changed

+101
-55
lines changed

1 file changed

+101
-55
lines changed

analysis/reanalyze/src/ReactiveSolver.ml

Lines changed: 101 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
(** Reactive dead code solver.
22
3-
Reactive pipeline: decls + live → dead_decls, live_decls, dead_modules, dead_decls_by_file
3+
Reactive pipeline: decls + live → dead_decls, live_decls, dead_modules, dead_decls_by_file, issues_by_file
44
55
Current status:
6-
- dead_decls, live_decls, dead_modules, dead_decls_by_file are all reactive
6+
- All collections are reactive (zero recomputation on cache hit for unchanged files)
7+
- dead_decls, live_decls = decls partitioned by liveness (reactive join)
78
- dead_modules = modules with dead decls but no live decls (reactive anti-join)
89
- dead_decls_by_file = dead decls grouped by file (reactive flatMap with merge)
10+
- issues_by_file = per-file issue generation (reactive flatMap)
911
- is_pos_live uses reactive live collection (no resolvedDead mutation)
1012
- shouldReport callback replaces report field mutation (no mutation needed)
11-
- Per-file issue generation using reactive dead_decls_by_file
1213
- isInsideReportedValue is per-file only, so files are independent
14+
- Module issues generated in collect_issues from dead_modules + modules_with_reported_values
1315
14-
TODO for fully reactive issues:
15-
- Make per-file issues a reactive collection (issues_by_file)
16-
Currently we iterate dead_decls_by_file to generate issues
16+
TODO for further optimization:
1717
- hasRefBelow: uses O(total_refs) linear scan of refs_from per dead decl;
1818
could use reactive refs_to index for O(1) lookup per decl
1919
@@ -33,6 +33,11 @@ type t = {
3333
(** Modules where all declarations are dead. Reactive anti-join. *)
3434
dead_decls_by_file: (string, Decl.t list) Reactive.t;
3535
(** Dead declarations grouped by file. Reactive per-file grouping. *)
36+
issues_by_file: (string, Issue.t list * Name.t list) Reactive.t;
37+
(** Dead code issues grouped by file. Reactive per-file issue generation.
38+
First component: value/type/exception issues.
39+
Second component: modules with at least one reported value (for module issue generation). *)
40+
config: DceConfig.t;
3641
}
3742

3843
(** Extract module name from a declaration *)
@@ -103,7 +108,53 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t)
103108
()
104109
in
105110

106-
{decls; live; dead_decls; live_decls; annotations; value_refs_from; dead_modules; dead_decls_by_file}
111+
(* hasRefBelow callback - captured once, uses current state of value_refs_from *)
112+
let transitive = config.DceConfig.run.transitive in
113+
let hasRefBelow =
114+
match value_refs_from with
115+
| None -> fun _ -> false
116+
| Some refs_from ->
117+
DeadCommon.make_hasRefBelow ~transitive ~iter_value_refs_from:(fun f ->
118+
Reactive.iter f refs_from)
119+
in
120+
121+
(* Reactive per-file issues - recomputed when dead_decls_by_file changes.
122+
Returns (file, (value_issues, modules_with_reported_values)) where
123+
modules_with_reported_values are modules that have at least one reported dead value.
124+
Module issues are generated separately in collect_issues using dead_modules. *)
125+
let issues_by_file =
126+
Reactive.flatMap dead_decls_by_file
127+
~f:(fun file decls ->
128+
(* Track modules that have reported values *)
129+
let modules_with_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 8 in
130+
(* shouldReport checks annotations reactively *)
131+
let shouldReport (decl : Decl.t) =
132+
match Reactive.get annotations decl.pos with
133+
| Some FileAnnotations.Live -> false
134+
| Some FileAnnotations.GenType -> false
135+
| Some FileAnnotations.Dead -> false
136+
| None -> true
137+
in
138+
(* Don't emit module issues here - track modules for later *)
139+
let checkModuleDead ~fileName:_ moduleName =
140+
Hashtbl.replace modules_with_values moduleName ();
141+
None (* Module issues generated separately *)
142+
in
143+
(* Sort within file and generate issues *)
144+
let sorted = decls |> List.fast_sort Decl.compareForReporting in
145+
let reporting_ctx = DeadCommon.ReportingContext.create () in
146+
let file_issues =
147+
sorted
148+
|> List.concat_map (fun decl ->
149+
DeadCommon.reportDeclaration ~config ~hasRefBelow ~checkModuleDead
150+
~shouldReport reporting_ctx decl)
151+
in
152+
let modules_list = Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values [] in
153+
[(file, (file_issues, modules_list))])
154+
()
155+
in
156+
157+
{decls; live; dead_decls; live_decls; annotations; value_refs_from; dead_modules; dead_decls_by_file; issues_by_file; config}
107158

108159
(** Check if a module is dead using reactive collection. Returns issue if dead.
109160
Uses reported_modules set to avoid duplicate reports. *)
@@ -126,13 +177,14 @@ let check_module_dead ~(dead_modules : (Name.t, Location.t) Reactive.t)
126177
Some (AnalysisResult.make_dead_module_issue ~loc ~moduleName)
127178
| None -> None
128179

129-
(** Collect issues from dead and live declarations.
130-
Uses reactive dead_modules instead of mutable DeadModules.
131-
O(dead_decls + live_decls), not O(all_decls). *)
180+
(** Collect issues from reactive issues_by_file.
181+
Only iterates the pre-computed reactive issues collection.
182+
Deduplicates module issues across files. *)
132183
let collect_issues ~(t : t) ~(config : DceConfig.t)
133184
~(ann_store : AnnotationStore.t) : Issue.t list =
185+
ignore config; (* config is stored in t *)
134186
let t0 = Unix.gettimeofday () in
135-
(* Track reported modules to avoid duplicates *)
187+
(* Track reported modules to avoid duplicates across files *)
136188
let reported_modules = Hashtbl.create 64 in
137189

138190
(* Check live declarations for incorrect @dead *)
@@ -155,59 +207,53 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
155207
t.live_decls;
156208
let t1 = Unix.gettimeofday () in
157209

158-
(* Generate issues per-file using reactive dead_decls_by_file.
159-
isInsideReportedValue only checks within same file, so files are independent. *)
160-
let transitive = config.DceConfig.run.transitive in
161-
let hasRefBelow =
162-
match t.value_refs_from with
163-
| None -> fun _ -> false
164-
| Some refs_from ->
165-
DeadCommon.make_hasRefBelow ~transitive ~iter_value_refs_from:(fun f ->
166-
Reactive.iter f refs_from)
167-
in
168-
(* Callback for checking dead modules using reactive collection *)
169-
let checkModuleDead ~fileName moduleName =
170-
check_module_dead ~dead_modules:t.dead_modules ~reported_modules ~fileName
171-
moduleName
172-
in
173-
(* Callback to check if we should report a dead decl (no mutation) *)
174-
let shouldReport (decl : Decl.t) =
175-
match Reactive.get t.annotations decl.pos with
176-
| Some FileAnnotations.Live -> false
177-
| Some FileAnnotations.GenType -> false
178-
| Some FileAnnotations.Dead -> false (* @dead = user knows, don't warn *)
179-
| None -> true
180-
in
210+
(* Collect issues from reactive issues_by_file *)
181211
let num_files = ref 0 in
182-
let dead_issues =
183-
let issues = ref [] in
184-
Reactive.iter
185-
(fun _file decls ->
186-
incr num_files;
187-
(* Sort within file for isInsideReportedValue - pass ALL decls for correct context *)
188-
let sorted = decls |> List.fast_sort Decl.compareForReporting in
189-
(* Fresh ReportingContext per file *)
190-
let reporting_ctx = DeadCommon.ReportingContext.create () in
191-
let file_issues =
192-
sorted
193-
|> List.concat_map (fun decl ->
194-
DeadCommon.reportDeclaration ~config ~hasRefBelow ~checkModuleDead
195-
~shouldReport reporting_ctx decl)
196-
in
197-
issues := file_issues @ !issues)
198-
t.dead_decls_by_file;
199-
!issues
200-
in
212+
let dead_issues = ref [] in
213+
(* Track modules that have at least one reported value (for module issue generation) *)
214+
let modules_with_reported_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 64 in
215+
Reactive.iter
216+
(fun _file (file_issues, modules_list) ->
217+
incr num_files;
218+
dead_issues := file_issues @ !dead_issues;
219+
(* Collect modules that have reported values *)
220+
List.iter
221+
(fun moduleName -> Hashtbl.replace modules_with_reported_values moduleName ())
222+
modules_list)
223+
t.issues_by_file;
201224
let t2 = Unix.gettimeofday () in
202225

226+
(* Generate module issues: only for modules that are dead AND have a reported value *)
227+
let module_issues = ref [] in
228+
let reported_modules : (Name.t, unit) Hashtbl.t = Hashtbl.create 64 in
229+
Reactive.iter
230+
(fun moduleName loc ->
231+
(* Only report if module has at least one reported dead value *)
232+
if Hashtbl.mem modules_with_reported_values moduleName then
233+
if not (Hashtbl.mem reported_modules moduleName) then (
234+
Hashtbl.replace reported_modules moduleName ();
235+
let loc =
236+
if loc.Location.loc_ghost then
237+
let pos_fname = loc.loc_start.pos_fname in
238+
let pos =
239+
{Lexing.pos_fname; pos_lnum = 0; pos_bol = 0; pos_cnum = 0}
240+
in
241+
{Location.loc_start = pos; loc_end = pos; loc_ghost = false}
242+
else loc
243+
in
244+
module_issues := AnalysisResult.make_dead_module_issue ~loc ~moduleName :: !module_issues))
245+
t.dead_modules;
246+
let t3 = Unix.gettimeofday () in
247+
203248
if !Cli.timing then
204249
Printf.eprintf
205-
" collect_issues: iter_live=%.2fms per_file=%.2fms (%d files)\n"
250+
" collect_issues: iter_live=%.2fms iter_issues=%.2fms iter_modules=%.2fms (%d files)\n"
206251
((t1 -. t0) *. 1000.0)
207252
((t2 -. t1) *. 1000.0)
253+
((t3 -. t2) *. 1000.0)
208254
!num_files;
209255

210-
List.rev !incorrect_dead_issues @ dead_issues
256+
List.rev !incorrect_dead_issues @ !module_issues @ !dead_issues
211257

212258
(** Iterate over live declarations *)
213259
let iter_live_decls ~(t : t) (f : Decl.t -> unit) : unit =

0 commit comments

Comments
 (0)