> On Thu, Jul 15, 2021 at 11:18:12AM -0700, Han Zhou wrote: > > On Wed, Jul 14, 2021 at 1:34 AM Lorenzo Bianconi < > > [email protected]> wrote: > > > This should avoid some work by doing the cheapest check (the one on > > > UseLogicalDatapathGroups) before any joins. DDlog is probably > > > factoring out the reference to the Flow relation, which is identical > > > in both, but this ought to avoid the group_by aggregation (which is > > > relatively expensive) in the case where UseLogicalDatapathGroups is > > > not enabled. > > > > Thanks! I didn't know that the order matters. (not sure if there is > > documentation that I missed) > > In general, DDlog executes each rule in the order given, so if you have > a series of joins in a rule, then it's a good idea to order them, if you > can, to keep the number of records small at each join step. It won't > affect the correctness, but it will affect performance. > > This might not be documented well. I do occasionally work on the DDlog > documentation, so I've made a note to try to see whether the effect of > ordering is mentioned and improve it if I can. > > In this particular case, I talked to Leonid about it during a discussion > of how to improve performance and memory use in the benchmark currently > at issue, and Leonid says that the ordering doesn't actually matter in > this case because both of the possibilities (the ones for > UseLogicalDatapathGroups[true] and UseLogicalDatapathGroups[false]) had > an identical clause at the beginning. DDlog optimizes identical > prefixes, so there was no real difference in performance. > > This patch could, therefore, be dropped, but it should also not be > harmful. Dropping it would require some changes to the later patch that > updates the ovn-northd ddlog code, so folding it into that one would > also be an option. >
Do you want me to repost dropping this patch or is the series fine as it is? Regards, Lorenzo _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
