From: Leonid Ryzhyk <[email protected]>
This patch is a workaround for a performance issue in the DDlog
compiler. The issue will hopefully be resolved in a future version of
DDlog, but for now we need this and possibly a few other similar fixes.
Here is one affected rule:
```
sb::Out_Port_Group(._uuid = hash128(sb_name), .name = sb_name, .ports =
port_names) :-
nb::Port_Group(._uuid = _uuid, .name = nb_name, .ports = pg_ports),
var port_uuid = FlatMap(pg_ports),
&SwitchPort(.lsp = lsp@nb::Logical_Switch_Port{._uuid = port_uuid,
.name = port_name},
.sw = &Switch{.ls = nb::Logical_Switch{._uuid = ls_uuid}}),
TunKeyAllocation(.datapath = ls_uuid, .tunkey = tunkey),
var sb_name = "${tunkey}_${nb_name}",
var port_names = port_name.group_by((_uuid, sb_name)).to_set().
```
The first literal in the body of the rule binds variable `pg_ports` to
the array of ports in the port group. This is a potentially large
array that immediately gets flattened by the `FlatMap` operator.
Since the `pg_ports` variable is not used in the remainder of the rule,
DDlog normally would not propagate it through the rest of the rule.
Unfortunately, due to a subtle semantic quirk, the behavior is different
when there is a `group_by` operator further down in the rule, in which
case unused variables are still propagated through the rule, which
involves expensive copies.
The workaround I implemented factors the first two terms in the
rule into a separate `PortGroupPort` relation, so that the ports array
no longer occurs in the new version of the rule:
```
sb::Out_Port_Group(._uuid = hash128(sb_name), .name = sb_name, .ports =
port_names) :-
PortGroupPort(.pg_uuid = _uuid, .pg_name = nb_name, .port = port_uuid),
&SwitchPort(.lsp = lsp@nb::Logical_Switch_Port{._uuid = port_uuid,
.name = port_name},
.sw = &Switch{.ls = nb::Logical_Switch{._uuid = ls_uuid}}),
TunKeyAllocation(.datapath = ls_uuid, .tunkey = tunkey),
var sb_name = "${tunkey}_${nb_name}",
var port_names = port_name.group_by((_uuid, sb_name)).to_set().
```
Again, benchmarking is likely to reveal more instances of this. A
proper fix will require a change to the DDlog compiler.
Signed-off-by: Leonid Ryzhyk <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
---
northd/ovn_northd.dl | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 51bbc832b7b6..50203de88867 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -720,11 +720,10 @@ sb::Out_Address_Set(._uuid = hash128("svc_monitor_mac"),
SvcMonitorMac(svc_monitor_mac).
sb::Out_Address_Set(hash128(as_name), as_name, pg_ip4addrs.union()) :-
- nb::Port_Group(.ports = pg_ports, .name = pg_name),
+ PortGroupPort(.pg_name = pg_name, .port = port_uuid),
var as_name = pg_name ++ "_ip4",
// avoid name collisions with user-defined Address_Sets
not nb::Address_Set(.name = as_name),
- var port_uuid = FlatMap(pg_ports),
PortStaticAddresses(.lsport = port_uuid, .ip4addrs = stat),
SwitchPortNewDynamicAddress(&SwitchPort{.lsp =
nb::Logical_Switch_Port{._uuid = port_uuid}},
dyn_addr),
@@ -746,11 +745,10 @@ sb::Out_Address_Set(hash128(as_name), as_name,
set_empty()) :-
not nb::Address_Set(.name = as_name).
sb::Out_Address_Set(hash128(as_name), as_name, pg_ip6addrs.union()) :-
- nb::Port_Group(.ports = pg_ports, .name = pg_name),
+ PortGroupPort(.pg_name = pg_name, .port = port_uuid),
var as_name = pg_name ++ "_ip6",
// avoid name collisions with user-defined Address_Sets
not nb::Address_Set(.name = as_name),
- var port_uuid = FlatMap(pg_ports),
PortStaticAddresses(.lsport = port_uuid, .ip6addrs = stat),
SwitchPortNewDynamicAddress(&SwitchPort{.lsp =
nb::Logical_Switch_Port{._uuid = port_uuid}},
dyn_addr),
@@ -779,9 +777,18 @@ sb::Out_Address_Set(hash128(as_name), as_name,
set_empty()) :-
* SB Port_Group.name uniqueness constraint, ovn-northd populates the field
* with the value: <SB.Logical_Datapath.tunnel_key>_<NB.Port_Group.name>.
*/
+
+relation PortGroupPort(
+ pg_uuid: uuid,
+ pg_name: string,
+ port: uuid)
+
+PortGroupPort(pg_uuid, pg_name, port) :-
+ nb::Port_Group(._uuid = pg_uuid, .name = pg_name, .ports = pg_ports),
+ var port = FlatMap(pg_ports).
+
sb::Out_Port_Group(._uuid = hash128(sb_name), .name = sb_name, .ports =
port_names) :-
- nb::Port_Group(._uuid = _uuid, .name = nb_name, .ports = pg_ports),
- var port_uuid = FlatMap(pg_ports),
+ PortGroupPort(.pg_uuid = _uuid, .pg_name = nb_name, .port = port_uuid),
&SwitchPort(.lsp = lsp@nb::Logical_Switch_Port{._uuid = port_uuid,
.name = port_name},
.sw = &Switch{.ls = nb::Logical_Switch{._uuid = ls_uuid}}),
--
2.31.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
