On Sat, Jun 6, 2015 at 2:55 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Mon, Jun 01, 2015 at 12:30:03AM -0700, Andy Zhou wrote:
>> Store ovsdb monitor in a global hmap. If a newly created ovsdb monitor
>> object monitors the same tables and columns as an existing one, the
>> existing monitor will be reused.
>>
>> With this patch, jsonrpc monitor and ovsdb monitor now have N:1 mapping.
>> The goals are to:
>> 1) Reduce the cost of maintaining duplicated monitors.
>> 2) Allow for create Json cache for the same updates. Json cache will be
>> introduced in the following patch.
>>
>> Signed-off-by: Andy Zhou <az...@nicira.com>
>
> In ovsdb_monitor_get_initial(), in the case where we find an existing
> changes object, do we still need to iterate over all the rows? I think
> that the changes have to already be composed properly, in that case.
>
> In the same function, I think that we could skip the second call to
> ovsdb_monitor_table_find_changes(), in the "if (!changes)" case, if
> ovsdb_monitor_table_add_changes() returned the new object.
>
> Acked-by: Ben Pfaff <b...@nicira.com>
Thanks. Both are good points. Pushed to master with the following
incremental changes folded in.
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 8b1c7d8..fb45ca6 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -111,8 +111,8 @@ struct ovsdb_monitor_table {
};
static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
-static void ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
- uint64_t next_txn);
+static struct ovsdb_monitor_changes * ovsdb_monitor_table_add_changes(
+ struct ovsdb_monitor_table *mt, uint64_t next_txn);
static struct ovsdb_monitor_changes *ovsdb_monitor_table_find_changes(
struct ovsdb_monitor_table *mt, uint64_t unflushed);
static void ovsdb_monitor_changes_destroy(
@@ -325,7 +325,7 @@ ovsdb_monitor_table_check_duplicates(struct
ovsdb_monitor *m,
return NULL;
}
-static void
+static struct ovsdb_monitor_changes *
ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
uint64_t next_txn)
{
@@ -338,6 +338,8 @@ ovsdb_monitor_table_add_changes(struct
ovsdb_monitor_table *mt,
changes->n_refs = 1;
hmap_init(&changes->rows);
hmap_insert(&mt->changes, &changes->hmap_node, hash_uint64(next_txn));
+
+ return changes;
};
static struct ovsdb_monitor_changes *
@@ -673,15 +675,13 @@ ovsdb_monitor_get_initial(const struct
ovsdb_monitor *dbmon)
changes = ovsdb_monitor_table_find_changes(mt, 0);
if (!changes) {
- ovsdb_monitor_table_add_changes(mt, 0);
- changes = ovsdb_monitor_table_find_changes(mt, 0);
+ changes = ovsdb_monitor_table_add_changes(mt, 0);
+ HMAP_FOR_EACH (row, hmap_node, &mt->table->rows) {
+ ovsdb_monitor_changes_update(NULL, row, mt, changes);
+ }
} else {
changes->n_refs++;
}
-
- HMAP_FOR_EACH (row, hmap_node, &mt->table->rows) {
- ovsdb_monitor_changes_update(NULL, row, mt, changes);
- }
}
}
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
