Skip to content

Commit

Permalink
[flow][indexing] Diff the new_available_exports_index and `old_avai…
Browse files Browse the repository at this point in the history
…lable_exports_index`

Summary:
This stack aims to fix the bug that incremental update of exports will incorrectly drop some export count to zero, because the current subtract function will incorrectly remove the entire export even through it will be added back.

This diff fixes the issue by first diffing the available export index into two disjoint index for additional and removal, which are then passes to `Export_index.merge` and `Export_index.subtract`. This ensures that for the overlapping old and new available index that's supposed to cancel out, they won't incorrectly nuke the usage count entry.

Changelog: [fix] Fixed a bug that causes incorrect updates to our index that tracks usage of exports, which leads to incorrect autoimport-ranked-by-usage results.

Reviewed By: panagosg7

Differential Revision: D69694640

fbshipit-source-id: 9f777f69db78d05b49b373f68531b1d777590763
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Feb 18, 2025
1 parent 81f82d4 commit fc43315
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
23 changes: 23 additions & 0 deletions src/services/export/index/export_index.ml
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,29 @@ let fold ~f ~init t =

let map ~f t = SMap.map (ExportMap.map f) t

let diff ~old_index ~new_index =
let exist_in_index export_name export_key map =
match SMap.find_opt export_name map with
| None -> false
| Some map -> ExportMap.mem export_key map
in
let diff_index l r =
SMap.fold
(fun export_name map acc ->
ExportMap.fold
(fun export_key _ acc ->
if exist_in_index export_name export_key r then
acc
else
let (s, k) = export_key in
add export_name s k acc)
map
acc)
l
SMap.empty
in
(diff_index new_index old_index, diff_index old_index new_index)

let subtract old_t t =
let (t, dead_names) =
SMap.fold
Expand Down
4 changes: 4 additions & 0 deletions src/services/export/index/export_index.mli
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ val merge : t -> t -> t

val merge_export_import : t -> t -> t

(* Returns tuple of two disjoint index: (addition_index, removal_index),
* to be passed to [merge] and `[subtract]`. *)
val diff : old_index:t -> new_index:t -> t * t

(** [subtract to_remove t] removes all of the exports in [to_remove] from [t], and
also returns a list of keys that no longer are exported by any file. *)
val subtract : t -> t -> t * string list
Expand Down
12 changes: 8 additions & 4 deletions src/services/export/search/export_search.ml
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,16 @@ let init index =
let type_matcher = Fuzzy_path.init types in
{ index; value_matcher; type_matcher }

let merge_available_exports old_index new_index { index; value_matcher; type_matcher } =
let (index, dead_candidates) = Export_index.subtract old_index index in
let merge_available_exports
old_available_exports_index new_available_exports_index { index; value_matcher; type_matcher } =
let (addition_index, removal_index) =
Export_index.diff ~old_index:old_available_exports_index ~new_index:new_available_exports_index
in
let (index, dead_candidates) = Export_index.subtract removal_index index in
let value_matcher = Fuzzy_path.remove_candidates value_matcher dead_candidates in
let type_matcher = Fuzzy_path.remove_candidates type_matcher dead_candidates in
let index = Export_index.merge new_index index in
let { values; types } = partition_candidates new_index in
let index = Export_index.merge addition_index index in
let { values; types } = partition_candidates addition_index in
let value_matcher = Fuzzy_path.add_candidates value_matcher values in
let type_matcher = Fuzzy_path.add_candidates type_matcher types in
{ index; value_matcher; type_matcher }
Expand Down

0 comments on commit fc43315

Please sign in to comment.