Skip to content

Commit fbc5bbc

Browse files
committed
Make incorrect @dead detection reactive
Add incorrect_dead_decls = reactive join of live_decls + annotations where annotation = Dead. Now we only iterate the few incorrect decls instead of scanning all ~17,500 live declarations. Timing improvement (benchmark): - incorrect_dead: 0.03ms (was ~5ms when scanning all live_decls) - dead_code total: ~7-10ms (was ~10-11ms) All 380/19000 issues still match between reactive and non-reactive modes.
1 parent 46012ef commit fbc5bbc

File tree

2 files changed

+76
-39
lines changed

2 files changed

+76
-39
lines changed

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,9 @@ let reportDeclaration ~config ~hasRefBelow ?checkModuleDead ?shouldReport
228228
let dead_module_issue =
229229
match checkModuleDead with
230230
| Some f -> f ~fileName:decl.pos.pos_fname moduleName
231-
| None -> DeadModules.checkModuleDead ~config ~fileName:decl.pos.pos_fname moduleName
231+
| None ->
232+
DeadModules.checkModuleDead ~config ~fileName:decl.pos.pos_fname
233+
moduleName
232234
in
233235
let dead_value_issue = makeDeadIssue ~decl ~message deadWarning in
234236
(* Return in order: dead module first (if any), then dead value *)

analysis/reanalyze/src/ReactiveSolver.ml

Lines changed: 73 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
(** Reactive dead code solver.
22
3-
Reactive pipeline: decls + live → dead_decls, live_decls, dead_modules, dead_decls_by_file, issues_by_file
3+
Reactive pipeline: decls + live + annotations → dead_decls, live_decls, dead_modules,
4+
dead_decls_by_file, issues_by_file, incorrect_dead_decls
45
56
Current status:
67
- All collections are reactive (zero recomputation on cache hit for unchanged files)
@@ -9,6 +10,7 @@
910
- dead_decls_by_file = dead decls grouped by file (reactive flatMap with merge)
1011
- value_refs_from_by_file = refs grouped by source file (reactive flatMap with merge)
1112
- issues_by_file = per-file issue generation (reactive flatMap)
13+
- incorrect_dead_decls = live decls with @dead annotation (reactive join)
1214
- is_pos_live uses reactive live collection (no resolvedDead mutation)
1315
- shouldReport callback replaces report field mutation (no mutation needed)
1416
- isInsideReportedValue is per-file only, so files are independent
@@ -35,6 +37,8 @@ type t = {
3537
(** Dead code issues grouped by file. Reactive per-file issue generation.
3638
First component: value/type/exception issues.
3739
Second component: modules with at least one reported value (for module issue generation). *)
40+
incorrect_dead_decls: (Lexing.position, Decl.t) Reactive.t;
41+
(** Live declarations with @dead annotation. Reactive join of live_decls + annotations. *)
3842
config: DceConfig.t;
3943
}
4044

@@ -112,12 +116,12 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t)
112116
match value_refs_from with
113117
| None -> None
114118
| Some refs_from ->
115-
Some (
116-
Reactive.flatMap refs_from
117-
~f:(fun posFrom posToSet -> [(posFrom.Lexing.pos_fname, [(posFrom, posToSet)])])
118-
~merge:(fun refs1 refs2 -> refs1 @ refs2)
119-
()
120-
)
119+
Some
120+
(Reactive.flatMap refs_from
121+
~f:(fun posFrom posToSet ->
122+
[(posFrom.Lexing.pos_fname, [(posFrom, posToSet)])])
123+
~merge:(fun refs1 refs2 -> refs1 @ refs2)
124+
())
121125
in
122126

123127
(* Reactive per-file issues - recomputed when dead_decls_by_file changes.
@@ -150,38 +154,64 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t)
150154
else
151155
match value_refs_from_by_file with
152156
| None -> fun _ -> false
153-
| Some refs_by_file ->
157+
| Some refs_by_file -> (
154158
let file_refs = Reactive.get refs_by_file file in
155159
fun decl ->
156160
match file_refs with
157161
| None -> false
158162
| Some refs_list ->
159-
List.exists (fun (posFrom, posToSet) ->
160-
PosSet.mem decl.Decl.pos posToSet &&
161-
DeadCommon.refIsBelow decl posFrom
162-
) refs_list
163+
List.exists
164+
(fun (posFrom, posToSet) ->
165+
PosSet.mem decl.Decl.pos posToSet
166+
&& DeadCommon.refIsBelow decl posFrom)
167+
refs_list)
163168
in
164169
(* Sort within file and generate issues *)
165170
let sorted = decls |> List.fast_sort Decl.compareForReporting in
166171
let reporting_ctx = DeadCommon.ReportingContext.create () in
167172
let file_issues =
168173
sorted
169174
|> List.concat_map (fun decl ->
170-
DeadCommon.reportDeclaration ~config ~hasRefBelow ~checkModuleDead
171-
~shouldReport reporting_ctx decl)
175+
DeadCommon.reportDeclaration ~config ~hasRefBelow
176+
~checkModuleDead ~shouldReport reporting_ctx decl)
177+
in
178+
let modules_list =
179+
Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values []
172180
in
173-
let modules_list = Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values [] in
174181
[(file, (file_issues, modules_list))])
175182
()
176183
in
177184

178-
{decls; live; dead_decls; live_decls; annotations; value_refs_from; dead_modules; dead_decls_by_file; issues_by_file; config}
185+
(* Reactive incorrect @dead: live decls with @dead annotation *)
186+
let incorrect_dead_decls =
187+
Reactive.join live_decls annotations
188+
~key_of:(fun pos _decl -> pos)
189+
~f:(fun pos decl ann_opt ->
190+
match ann_opt with
191+
| Some FileAnnotations.Dead -> [(pos, decl)]
192+
| _ -> [])
193+
()
194+
in
195+
196+
{
197+
decls;
198+
live;
199+
dead_decls;
200+
live_decls;
201+
annotations;
202+
value_refs_from;
203+
dead_modules;
204+
dead_decls_by_file;
205+
issues_by_file;
206+
incorrect_dead_decls;
207+
config;
208+
}
179209

180210
(** Check if a module is dead using reactive collection. Returns issue if dead.
181211
Uses reported_modules set to avoid duplicate reports. *)
182212
let check_module_dead ~(dead_modules : (Name.t, Location.t) Reactive.t)
183-
~(reported_modules : (Name.t, unit) Hashtbl.t) ~fileName:pos_fname moduleName
184-
: Issue.t option =
213+
~(reported_modules : (Name.t, unit) Hashtbl.t) ~fileName:pos_fname
214+
moduleName : Issue.t option =
185215
if Hashtbl.mem reported_modules moduleName then None
186216
else
187217
match Reactive.get dead_modules moduleName with
@@ -203,43 +233,45 @@ let check_module_dead ~(dead_modules : (Name.t, Location.t) Reactive.t)
203233
Deduplicates module issues across files. *)
204234
let collect_issues ~(t : t) ~(config : DceConfig.t)
205235
~(ann_store : AnnotationStore.t) : Issue.t list =
206-
ignore config; (* config is stored in t *)
236+
ignore (config, ann_store);
237+
(* config is stored in t, ann_store used via reactive annotations *)
207238
let t0 = Unix.gettimeofday () in
208239
(* Track reported modules to avoid duplicates across files *)
209240
let reported_modules = Hashtbl.create 64 in
210241

211-
(* Check live declarations for incorrect @dead *)
242+
(* Collect incorrect @dead issues from reactive collection *)
212243
let incorrect_dead_issues = ref [] in
213244
Reactive.iter
214245
(fun _pos (decl : Decl.t) ->
215-
(* Check for incorrect @dead annotation on live decl *)
216-
if AnnotationStore.is_annotated_dead ann_store decl.pos then (
217-
let issue =
218-
DeadCommon.makeDeadIssue ~decl
219-
~message:" is annotated @dead but is live"
220-
Issue.IncorrectDeadAnnotation
221-
in
222-
(* Check if module is dead using reactive collection *)
223-
check_module_dead ~dead_modules:t.dead_modules ~reported_modules
224-
~fileName:decl.pos.pos_fname (decl_module_name decl)
225-
|> Option.iter (fun mod_issue ->
226-
incorrect_dead_issues := mod_issue :: !incorrect_dead_issues);
227-
incorrect_dead_issues := issue :: !incorrect_dead_issues))
228-
t.live_decls;
246+
let issue =
247+
DeadCommon.makeDeadIssue ~decl
248+
~message:" is annotated @dead but is live"
249+
Issue.IncorrectDeadAnnotation
250+
in
251+
(* Check if module is dead using reactive collection *)
252+
check_module_dead ~dead_modules:t.dead_modules ~reported_modules
253+
~fileName:decl.pos.pos_fname (decl_module_name decl)
254+
|> Option.iter (fun mod_issue ->
255+
incorrect_dead_issues := mod_issue :: !incorrect_dead_issues);
256+
incorrect_dead_issues := issue :: !incorrect_dead_issues)
257+
t.incorrect_dead_decls;
229258
let t1 = Unix.gettimeofday () in
230259

231260
(* Collect issues from reactive issues_by_file *)
232261
let num_files = ref 0 in
233262
let dead_issues = ref [] in
234263
(* Track modules that have at least one reported value (for module issue generation) *)
235-
let modules_with_reported_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 64 in
264+
let modules_with_reported_values : (Name.t, unit) Hashtbl.t =
265+
Hashtbl.create 64
266+
in
236267
Reactive.iter
237268
(fun _file (file_issues, modules_list) ->
238269
incr num_files;
239270
dead_issues := file_issues @ !dead_issues;
240271
(* Collect modules that have reported values *)
241272
List.iter
242-
(fun moduleName -> Hashtbl.replace modules_with_reported_values moduleName ())
273+
(fun moduleName ->
274+
Hashtbl.replace modules_with_reported_values moduleName ())
243275
modules_list)
244276
t.issues_by_file;
245277
let t2 = Unix.gettimeofday () in
@@ -262,13 +294,16 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
262294
{Location.loc_start = pos; loc_end = pos; loc_ghost = false}
263295
else loc
264296
in
265-
module_issues := AnalysisResult.make_dead_module_issue ~loc ~moduleName :: !module_issues))
297+
module_issues :=
298+
AnalysisResult.make_dead_module_issue ~loc ~moduleName
299+
:: !module_issues))
266300
t.dead_modules;
267301
let t3 = Unix.gettimeofday () in
268302

269303
if !Cli.timing then
270304
Printf.eprintf
271-
" collect_issues: iter_live=%.2fms iter_issues=%.2fms iter_modules=%.2fms (%d files)\n"
305+
" collect_issues: incorrect_dead=%.2fms iter_issues=%.2fms \
306+
iter_modules=%.2fms (%d files)\n"
272307
((t1 -. t0) *. 1000.0)
273308
((t2 -. t1) *. 1000.0)
274309
((t3 -. t2) *. 1000.0)

0 commit comments

Comments
 (0)