From: Leonid Ryzhyk <lryz...@vmware.com> 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 <lryz...@vmware.com> Signed-off-by: Ben Pfaff <b...@ovn.org> --- 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 80d8598bd7dc..5a7a11295964 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -712,11 +712,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), @@ -738,11 +737,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), @@ -771,9 +769,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.29.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev