On 9/22/21 13:16, Dumitru Ceara wrote: > On 9/22/21 1:14 PM, Dumitru Ceara wrote: >> On 9/16/21 10:15 PM, Ilya Maximets wrote: >>> Currently, even if one reference added to the set of strong references >>> or removed from it, ovsdb-server will walk through the whole set and >>> re-count references to other rows. These referenced rows will also be >>> added to the transaction in order to re-count their references. >>> >>> For example, every time Logical Switch Port added to a Logical Switch, >>> OVN Northbound database server will walk through all ports of this >>> Logical Switch, clone their rows, and re-count references. This is >>> not very efficient. Instead, it can only increase reference counters >>> for added references and reduce for removed ones. In many cases this >>> will be only one row affected in the Logical_Switch_Port table. >>> >>> Introducing new function that generates a diff of two datum objects, >>> but stores added and removed atoms separately, so they can be used >>> to increase or decrease row reference counters accordingly. >>> >>> This change allows to perform several times more transactions that >>> adds or removes strong references to/from sets per second, because >>> ovsdb-server no longer clones and re-counts rows that are irrelevant >>> to current transaction. >>> >>> Signed-off-by: Ilya Maximets <[email protected]> >>> --- >> >> Hi Ilya, >> >> I've been trying this patch out and with a micro benchmark that does: >> >> - create X rows in table A >> - create Y rows in table B (non-root) and reference the rows in table B > > Sorry, I meant create Y rows for each row in X here, essentially X * Y > rows in table B. > >> from exactly one row in table A, uniformly distributed, one new row per >> transaction. >> - remove the references to all rows in B from table A, one removal per >> transaction. >> >> I got the following results: >> >> X=120, Y=250, w/o patch: >> - ovsdb-server CPU 10.4%, 27 sec >> >> X=120, Y=250, w/ patch (gain ~33%): >> - ovsdb-server CPU 6.9%, 18 sec >> >> X=1, Y=30000, w/o patch: >> - ovsdb-server CPU goes up to 100% and I stopped it after ~15 min while >> it was still executing the "create" part of the test. >> >> X=1, Y=30000, w/ patch: >> - ovsdb-server CPU 43.2%, 175 sec >> >> I was wondering if it makes sense to factor out the common part of >> ovsdb_datum_added_removed() and ovsdb_datum_diff(). It seems to me that >> ovsdb_datum_diff(diff, old, new, type) is almost the same as >> ovsdb_datum_added_removed(diff, diff, old, new, type), we just need to >> handle the non-composite types separately and deal with the case when >> 'added' == 'new' if we detect a value changing for a given key.
I thought about this and it would be great to reduce some code duplication. But I found dealing with corner cases inside he function a bit awkward. Especially the part where we need to check that 'added' == 'new' and add only one of the values to one of the sets and not touch the other one. So, I kept it as is for now. >> >> In any case, the code looks OK to me in the current form too, so: >> >> Acked-by: Dumitru Ceara <[email protected]> Applied to master. Thanks! Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
