On Thu, Jul 15, 2021 at 08:45:00PM +0200, Lorenzo Bianconi wrote: > > 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?
The series is fine as is. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
