-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Rewrite maps, tuples, lists as BDDs. Performance improvements. #14693
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.
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
Conversation
- Updated the representation of non-empty lists and empty intersections/differences to use BDD structures. - Introduced new functions for BDD operations including list_get, list_get_pos, and list_all? for better handling of BDD paths. - Modified list normalization and difference functions to work with BDDs
…alize` and `fun_get`
Note: I haven't reviewed the code or investigated properly yet, but I tried the branch on a project and noticed:
defguard is_doc(doc)
when is_list(doc) or is_binary(doc) or doc == :doc_line or
(is_tuple(doc) and elem(doc, 0) in @docs) Removing one element from |
I beleive the issue is the usage of comparison in the BDD. For example, maps only use comparisons up to 32 terms, because from them on, the linear time of each comparison becomes too high. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that to_quoted
can now be very verbose for unions since it adds these and not
- technically correct but not very useful - which I'm worried might harm readability:
union(
open_map(a: number(), b: binary()),
open_map(a: integer())
)
# 1.18
%{..., a: float() or integer()}
# this branch
%{..., a: float() or integer(), b: binary()} or
(%{..., a: integer()} and not %{..., a: float() or integer(), b: binary()})
Co-authored-by: Jean Klingler <[email protected]>
Co-authored-by: Jean Klingler <[email protected]>
This is a drawback of the BDD representation. It is possible, I think, to get rid of those by using a more sophisticated normalization algorithm for printing (that I implemented in the past for a prototype, but not for this version). |
there is now map_dnf_fetch_static to avoid calling map_bdd_get in a loop. also, renamed map_bdd_get to map_bdd_to_dnf (avoids confusion with get operation on maps)
After reporting #14790 I checked with this branch merged to
and add this to the router under
Even with 3 resources it's extra slow. Both compilation and verifying takes a long time: when using sufficiently small number of routes and waiting long enough, after I am on Erlang 27.3.4.2 I hope this helps, I am sorry I was not able to narrow it down further 🙏🏻 |
I can reproduce this. I seems it's stuck trying to type %{method: method, path_info: path_info, host: host} = conn = prepare(conn)
decoded = Enum.map(path_info, &URI.decode/1)
case __match_route__(decoded, method, host) do
{metadata, prepare, pipeline, plug_opts} ->
Phoenix.Router.__call__(conn, metadata, prepare, pipeline, plug_opts)
:error ->
:erlang.error(Phoenix.Router.NoRouteError.exception(conn: conn, router: FooWeb.Router))
end I seems to spend a lot of time in Module.Types.Descr.pub_map_line_empty?(
:closed,
%{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.EpsilonController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{atom: {:union, %{api: [], require_authenticated_user: []}}}, %{bitmap: 2}},
:bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.OmegaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{new: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.OmegaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{index: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.OmegaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.PiController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{new: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.PiController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{index: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.PiController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.DeltaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{new: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.DeltaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{index: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.DeltaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.GammaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{new: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.GammaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{index: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.GammaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.AlphaModelController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{index: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.AlphaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{new: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.AlphaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{index: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.AlphaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.BetaController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list:
{{%{
atom: {:union, %{api: [], require_authenticated_user: []}}
}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{list_checked_out: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.UserSessionController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list: {{%{atom: {:union, %{browser: []}}}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{new: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.UserSessionController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list: {{%{atom: {:union, %{browser: []}}}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{delete: []}}}
},
closed: %{
log: %{atom: {:union, %{debug: []}}},
plug: %{atom: {:union, %{FooWeb.UserSessionController => []}}},
conn: %{atom: {:union, %{nil: []}}},
path_params: %{map: {{:closed, %{}}, :bdd_top, :bdd_bot}},
route: %{bitmap: 1},
pipe_through: %{
list: {{%{atom: {:union, %{browser: []}}}, %{bitmap: 2}}, :bdd_top, :bdd_bot}
},
plug_opts: %{atom: {:union, %{create: []}}}
}
) |
The key change is that map_line_empty was checking the whole tree no matter what, due to an or condition that should not be checked as the previous Enum.all? are enough to conclude. Also clarified the invariant that allows not to check the positives fields as a base case (still in map_line_empty?)
💚 💙 💜 💛 ❤️ |
Changing the representation of maps, tuples to instead use trees (binary decision diagrams).
As we use more and more the difference operator in the type system (e.g. to compute
fun_from_inferred_clauses(args_clauses)
for inferred function types from patterns), the "DNF" representation started running into pathological cases and hog out time during emptiness checks, and printing.The BDD is now used for set operations and subtyping, and translated (using previous logic and simplications) to DNF for printing and type operations like map fetch.
The BDD representation is common to maps, tuples, lists and functions. Some functions are generic (called bdd_difference, bdd_intersection, ...).
We also changes some algorithms:
tuple_eliminate_negation
was not generating disjoint tuples. It now does, which greatly improves performance and printing quality.-
tuple_empty?
now uses the same logic, this improves performances.pivot_overlapping_clauses
algorithm was not generating disjoint tuples. It now does.apply_disjoint
, when the functions are disjoint and non-empty (as produced by fun_from_inferred_clauses, for instance), which is much faster.