Skip to content

Use a map to memoize looked-up items in signatures #1049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Use a map to memoize looked-up items in signatures #1049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions src/xref2/component.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ module IdentMap = Map.Make (struct
let compare = Ident.compare
end)

module StringMap = struct
include Map.Make (String)

let add_multi k v m =
let v' = try find k m with Not_found -> [] in
add k (v :: v') m
end

module Delayed = struct
let eager = ref false

Expand Down Expand Up @@ -335,12 +343,24 @@ and Signature : sig

type t = {
items : item list;
mutable lookup_cache : item list StringMap.t;
compiled : bool;
removed : removed_item list;
doc : CComment.docs;
}
end =
Signature

val make :
item list -> compiled:bool -> removed_item list -> CComment.docs -> t

val update_items : t -> item list -> t
end = struct
include Signature

let make items ~compiled removed doc =
{ items; lookup_cache = StringMap.empty; compiled; removed; doc }

let update_items sg items = { sg with items; lookup_cache = StringMap.empty }
end

and Open : sig
type t = { expansion : Signature.t; doc : CComment.docs }
Expand Down Expand Up @@ -2497,7 +2517,7 @@ module Of_Lang = struct
sg.items
|> List.rev
in
{ items; removed = []; compiled = sg.compiled; doc = docs ident_map sg.doc }
Signature.make items ~compiled:sg.compiled [] (docs ident_map sg.doc)

and block_element _ b :
CComment.block_element Odoc_model.Comment.with_location =
Expand Down
14 changes: 14 additions & 0 deletions src/xref2/component.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ module PathClassTypeMap : Map.S with type key = Ident.path_class_type

module IdentMap : Map.S with type key = Ident.any

module StringMap : sig
include Map.S with type key = string

val add_multi : key -> 'a -> 'a list t -> 'a list t
(** Add a binding to a ['a list]. If the binding already exists in
the map, the new value is appended to the corresponding list instead. *)
end

(** Delayed is a bit like Lazy.t but may in the future offer the chance to peek inside
to be able to optimize the calculation *)
module Delayed : sig
Expand Down Expand Up @@ -299,10 +307,16 @@ and Signature : sig

type t = {
items : item list;
mutable lookup_cache : item list StringMap.t;
compiled : bool;
removed : removed_item list;
doc : CComment.docs;
}

val make :
item list -> compiled:bool -> removed_item list -> CComment.docs -> t

val update_items : t -> item list -> t
end

and Open : sig
Expand Down
112 changes: 78 additions & 34 deletions src/xref2/find.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,70 @@ let rec find_map f = function
| hd :: tl -> ( match f hd with Some _ as x -> x | None -> find_map f tl)
| [] -> None

let find_in_sig sg f =
let rec inner f = function
| Signature.Include i :: tl -> (
match inner f i.Include.expansion_.items with
| Some _ as x -> x
| None -> inner f tl)
| hd :: tl -> ( match f hd with Some _ as x -> x | None -> inner f tl)
| [] -> None
in
inner f sg.Signature.items

let filter_in_sig sg f =
let rec inner f = function
| Signature.Include i :: tl ->
inner f i.Include.expansion_.items @ inner f tl
| hd :: tl -> (
match f hd with Some x -> x :: inner f tl | None -> inner f tl)
| [] -> []
in
inner f sg.Signature.items
let rec filter_map f = function
| hd :: tl -> (
match f hd with Some x -> x :: filter_map f tl | None -> filter_map f tl)
| [] -> []

module Linear_lookup = struct
let find_in_sig :
Signature.t ->
(Signature.item -> 'a option) ->
(Signature.item * 'a) option =
fun sg f ->
let rec inner f = function
| Signature.Include i :: tl -> (
match inner f i.expansion_.items with
| Some _ as x -> x
| None -> inner f tl)
| hd :: tl -> (
match f hd with Some x -> Some (hd, x) | None -> inner f tl)
| [] -> None
in
inner f sg.items

let filter_in_sig :
Signature.t -> (Signature.item -> 'a option) -> (Signature.item * 'a) list
=
fun sg f ->
let rec inner f = function
| Signature.Include i :: tl -> inner f i.expansion_.items @ inner f tl
| hd :: tl -> (
match f hd with Some x -> (hd, x) :: inner f tl | None -> inner f tl)
| [] -> []
in
inner f sg.items
end

module Cached_lookup = struct
let find_in_sig :
Signature.t -> string -> (Signature.item -> 'a option) -> 'a option =
fun sg name f ->
try find_map f (StringMap.find name sg.lookup_cache)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With @gpetiot we found a bug here:

Say a signature contains both a module and a module type named Foo. A first lookup to the module would populate the cache with only the module. A second lookup to signature_in_sig would find something in the cache and never go through the linear lookup to find the module type.

with Not_found -> (
match Linear_lookup.find_in_sig sg f with
| None -> None
| Some (item, res) ->
sg.lookup_cache <- StringMap.add_multi name item sg.lookup_cache;
Some res)

let filter_in_sig :
Signature.t -> string -> (Signature.item -> 'a option) -> 'a list =
fun sg name f ->
try filter_map f (StringMap.find name sg.lookup_cache)
with Not_found -> (
match Linear_lookup.filter_in_sig sg f with
| [] -> []
| items_and_res ->
let items, res = List.split items_and_res in
sg.lookup_cache <-
List.fold_left
(fun acc item -> StringMap.add_multi name item acc)
sg.lookup_cache items;
res)
end

open Cached_lookup

(** Returns the last element of a list. Used to implement [_unambiguous]
functions. *)
Expand All @@ -90,19 +134,19 @@ let rec disambiguate = function
| _ :: tl -> disambiguate tl

let module_in_sig sg name =
find_in_sig sg (function
find_in_sig sg name (function
| Signature.Module (id, _, m) when N.module_ id = name ->
Some (`FModule (N.typed_module id, Delayed.get m))
| _ -> None)

let module_type_in_sig sg name =
find_in_sig sg (function
find_in_sig sg name (function
| Signature.ModuleType (id, mt) when N.module_type id = name ->
Some (`FModuleType (N.typed_module_type id, Delayed.get mt))
| _ -> None)

let type_in_sig sg name =
find_in_sig sg (function
find_in_sig sg name (function
| Signature.Type (id, _, m) when N.type_ id = name ->
Some (`FType (N.type' id, Delayed.get m))
| Class (id, _, c) when N.class_ id = name ->
Expand Down Expand Up @@ -157,13 +201,13 @@ let careful_type_in_sig sg name =
| None -> removed_type_in_sig sg name

let datatype_in_sig sg name =
find_in_sig sg (function
find_in_sig sg name (function
| Signature.Type (id, _, t) when N.type_ id = name ->
Some (`FType (N.type' id, Component.Delayed.get t))
| _ -> None)

let class_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Class (id, _, c) when N.class_ id = name ->
Some (`FClass (N.class' id, c))
| Signature.ClassType (id, _, c) when N.class_type id = name ->
Expand Down Expand Up @@ -219,7 +263,7 @@ let any_in_comment d name =
inner d

let any_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Module (id, _, m) when N.module_ id = name ->
Some (`FModule (N.typed_module id, Delayed.get m))
| ModuleSubstitution (id, ms) when N.module_ id = name ->
Expand Down Expand Up @@ -247,21 +291,21 @@ let any_in_sig sg name =
| _ -> None)

let signature_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Module (id, _, m) when N.module_ id = name ->
Some (`FModule (N.typed_module id, Delayed.get m))
| ModuleType (id, mt) when N.module_type id = name ->
Some (`FModuleType (N.typed_module_type id, Delayed.get mt))
| _ -> None)

let module_type_in_sig sg name =
find_in_sig sg (function
find_in_sig sg name (function
| Signature.ModuleType (id, m) when N.module_type id = name ->
Some (`FModuleType (N.typed_module_type id, Delayed.get m))
| _ -> None)

let value_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Value (id, m)
when N.value id = name || N.value id = "(" ^ name ^ ")" ->
(* For operator, the value will have name [(<op>)]. We match that even
Expand All @@ -272,12 +316,12 @@ let value_in_sig sg name =
let value_in_sig_unambiguous sg name = disambiguate (value_in_sig sg name)

let label_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Comment (`Docs d) -> any_in_comment d name
| _ -> None)

let exception_in_sig sg name =
find_in_sig sg (function
find_in_sig sg name (function
| Signature.Exception (id, e) when N.exception_ id = name ->
Some (`FExn (N.typed_exception id, e))
| _ -> None)
Expand All @@ -288,12 +332,12 @@ let extension_in_sig sg name =
| _ :: tl -> inner t tl
| [] -> None
in
find_in_sig sg (function
find_in_sig sg name (function
| Signature.TypExt t -> inner t t.Extension.constructors
| _ -> None)

let label_parent_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Module (id, _, m) when N.module_ id = name ->
Some (`FModule (N.typed_module id, Component.Delayed.get m))
| ModuleType (id, mt) when N.module_type id = name ->
Expand All @@ -307,7 +351,7 @@ let label_parent_in_sig sg name =
| _ -> None)

let any_in_type_in_sig sg name =
filter_in_sig sg (function
filter_in_sig sg name (function
| Signature.Type (id, _, t) -> (
let t = Delayed.get t in
match any_in_type t name with
Expand Down
2 changes: 1 addition & 1 deletion src/xref2/strengthen.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ and sig_items prefix ?canonical sg =
(item :: items, s))
([], []) sg.items
in
({ sg with items = List.rev items }, ids)
(update_items sg (List.rev items), ids)

and module_ :
?canonical:Odoc_model.Paths.Path.Module.t ->
Expand Down
21 changes: 12 additions & 9 deletions src/xref2/subst.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,15 +1004,16 @@ and rename_bound_idents s sg =
rest
| Include ({ expansion_; _ } as i) :: rest ->
let s, items = rename_bound_idents s [] expansion_.items in
rename_bound_idents s
(Include { i with expansion_ = { expansion_ with items; removed = [] } }
:: sg)
rest
let expansion_ =
make items ~compiled:expansion_.compiled [] expansion_.doc
in
rename_bound_idents s (Include { i with expansion_ } :: sg) rest
| Open { expansion; doc } :: rest ->
let s, items = rename_bound_idents s [] expansion.items in
rename_bound_idents s
(Open { expansion = { expansion with items; removed = [] }; doc } :: sg)
rest
let expansion =
make items ~compiled:expansion.compiled [] expansion.doc
in
rename_bound_idents s (Open { expansion; doc } :: sg) rest
| (Comment _ as item) :: rest -> rename_bound_idents s (item :: sg) rest

and removed_items s items =
Expand All @@ -1031,11 +1032,13 @@ and removed_items s items =
and signature s sg =
let s, items = rename_bound_idents s [] sg.items in
let items, removed, dont_recompile = apply_sig_map s items sg.removed in
{ sg with items; removed; compiled = sg.compiled && dont_recompile }
let compiled = sg.compiled && dont_recompile in
Component.Signature.make items ~compiled removed sg.doc

and apply_sig_map_sg s (sg : Component.Signature.t) =
let items, removed, dont_recompile = apply_sig_map s sg.items sg.removed in
{ sg with items; removed; compiled = sg.compiled && dont_recompile }
let compiled = sg.compiled && dont_recompile in
Component.Signature.make items ~compiled removed sg.doc

and apply_sig_map_item s item =
let open Component.Signature in
Expand Down
Loading