On 8/30/21 12:54 PM, Dumitru Ceara wrote:
> On 8/24/21 9:00 PM, Ilya Maximets wrote:
>> Same json from a json cache is typically sent to all the clients,
>> e.g., in case of OVN deployment with ovn-monitor-all=true.
>>
>> There could be hundreds or thousands connected clients and ovsdb
>> will serialize the same json object for each of them before sending.
>>
>> Serializing it once before storing into json cache to speed up
>> processing.
>>
>> This change allows to save a lot of CPU cycles and a bit of memory
>> since we need to store in memory only a string and not the full json
>> object.
>>
>> Testing with ovn-heater on 120 nodes using density-heavy scenario
>> shows reduction of the total CPU time used by Southbound DB processes
>> from 256 minutes to 147.  Duration of unreasonably long poll intervals
>> also reduced dramatically from 7 to 2 seconds:
>>
>>            Count   Min    Max   Median    Mean   95 percentile
>>  -------------------------------------------------------------
>>   Before   1934   1012   7480   4302.5   4875.3     7034.3
>>   After    1909   1004   2730   1453.0   1532.5     2053.6
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>> ---
>>  ovsdb/monitor.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
>> index 532dedcb6..ab814cf20 100644
>> --- a/ovsdb/monitor.c
>> +++ b/ovsdb/monitor.c
>> @@ -1231,6 +1231,15 @@ ovsdb_monitor_get_update(
>>                                              condition,
>>                                              
>> ovsdb_monitor_compose_row_update2);
>>                  if (!condition || !condition->conditional) {
>> +                    if (json) {
>> +                        struct json *json_serialized;
>> +
>> +                        /* Pre-serializing the object to avoid doing this
>> +                         * for every client. */
>> +                        json_serialized = 
>> json_serialized_object_create(json);
>> +                        json_destroy(json);
>> +                        json = json_serialized;
>> +                    }
> 
> I think this could be part of ovsdb_monitor_json_cache_insert()
> directly.  It would save a level of indentation here and it seems to me
> like it's just an implementation decision about how to store the cached
> json.

Not really.  ovsdb_monitor_get_update() returns the json object.
If serialization will be performed inside the ovsdb_monitor_json_cache_insert(),
we will return non-serialized version here and it will be serialized again
while sending to the first client.  Swapping of the 'json' object with the
serialized one here saves the processing time, i.e. saves one extra 
serialization.

> 
> Either way:
> 
> Acked-by: Dumitru Ceara <[email protected]>

Thanks!

> 
>>                      ovsdb_monitor_json_cache_insert(dbmon, version, mcs,
>>                                                      json);
>>                  }
>>
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to