[ovs-dev] [ovsdb speedup v3 19/19] ovsdb-monitor: add json cache

[Andy Zhou]

On Fri, May 29, 2015 at 1:05 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Thu, Apr 09, 2015 at 06:40:28PM -0700, Andy Zhou wrote:
>> Although multiple jsonrpc monitor can share the same ovsdb monitor, each
>> change still needs to transalted into json object from scratch. This
>> can be wastful is mutiple jsonrpc monitors are interested in the
>> same changes.
>>
>> Json chche impoves this by keeping an copy of json object generated
>> for transaction X to current transaction. When jsonrpc is interested
>> in a change, the cache is lookup first, if an json object is found,
>> a copy of it is handed back, skipping the regeneration process.
>>
>> Any commit to the monitor will empty the cache. since none of them
>> will be useful anymore.
>>
>> Signed-off-by: Andy Zhou <azhou at nicira.com>
>>
>> ---
>> v1->v2: Each monitor now has its own json cache. Remove the global
>>         json cache.
>>         Some refatoring to make to code easier to follow.
>>
>> v2->v3: Destory json_cache hmap on monitor destroy. Without it,
>>         valgrind reports memory leak.
>
> Some typos in commit message:
> s/transalted/translated/
> s/wastful is/wasteful if/
> s/chche/cache/
> s/lookup/searched/
>
> There's an indentation error here in ovsdb_monitor_json_cache_search():
> +        if (node->from_txn == from_txn) {
> +                return node;
> +        }
>
All fixed.

> The current implementation of ovsdb_monitor_commit() throws away the
> cache whenever there is any commit to the database.  However, most
> monitors do not every table and every column.  Do you think it would be
> worthwhile flushing the cache only when a column that is included in the
> monitor changes?
>
This is a good idea.  However, I don't see a way to easily extend current patch
to add this patch. So I sent out v4 without this enhancement.

I think we need the following two enhancements to support this feature:

1. For each transaction to only send the columns that are changed.
Actually, this can be
an standalone enhancement.

2. A monitor should be able to quickly identify if a transaction
applies.  I am thinking of
using a bit map for each table to represent column membership, to
allow for quick set
intersection calculation.

Make sense?  If yes, I will work on this as separate patch sets.

> Acked-by: Ben Pfaff <blp at nicira.com>
Thanks.