On Thu, Aug 31, 2017 at 09:50:08AM +0300, Liran Schour wrote:
> [email protected] wrote on 30/08/2017 07:33:14 PM:
> 
> > This removes n_true_cnd from struct ovsdb_monitor_session_condition.
> > It was an "optimization" that is not part of any inner loop, but
> > make the code harder to reason about.
> > 
> > Signed-off-by: Ben Pfaff <[email protected]>
> > ---
> >  ovsdb/monitor.c | 28 ++++++++++++----------------
> >  1 file changed, 12 insertions(+), 16 deletions(-)
> > 
> > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> > index 7a5c2f905560..c0f9c557ab67 100644
> > --- a/ovsdb/monitor.c
> > +++ b/ovsdb/monitor.c
> > @@ -46,8 +46,7 @@ static struct hmap ovsdb_monitors = 
> > HMAP_INITIALIZER(&ovsdb_monitors);
> > 
> >  /* Keep state of session's conditions */
> >  struct ovsdb_monitor_session_condition {
> > -    bool conditional;
> > -    size_t n_true_cnd;
> > +    bool conditional;        /* True iff every table's condition is 
> true. */
> >      struct shash tables;     /* Contains
> >                                *   "struct 
> > ovsdb_monitor_table_condition *"s. */
> >  };
> > @@ -583,8 +582,17 @@ static inline void
> >  ovsdb_monitor_session_condition_set_mode(
> >                                    struct 
> > ovsdb_monitor_session_condition *cond)
> >  {
> > -    cond->conditional = shash_count(&cond->tables) !=
> > -        cond->n_true_cnd;
> > +    struct shash_node *node;
> > +
> > +    SHASH_FOR_EACH (node, &cond->tables) {
> > +        struct ovsdb_monitor_table_condition *mtc = node->data;
> > +
> > +        if (!ovsdb_condition_is_true(&mtc->new_condition)) {
> > +            cond->conditional = true;
> > +            return;
> > +        }
> > +    }
> > +    cond->conditional = false;
> >  }
> > 
> >  /* Returnes an empty allocated session's condition state holder */
> > @@ -649,9 +657,6 @@ ovsdb_monitor_table_condition_create(
> >      shash_add(&condition->tables, table->schema->name, mtc);
> >      /* On session startup old == new condition */
> >      ovsdb_condition_clone(&mtc->new_condition, &mtc->old_condition);
> > -    if (ovsdb_condition_is_true(&mtc->old_condition)) {
> > -        condition->n_true_cnd++;
> > -    }
> >      ovsdb_monitor_session_condition_set_mode(condition);
> > 
> >      return NULL;
> > @@ -722,15 +727,6 @@ ovsdb_monitor_table_condition_updated(struct 
> > ovsdb_monitor_table *mt,
> >          /* If conditional monitoring - set old condition to new 
> condition */
> >          if (ovsdb_condition_cmp_3way(&mtc->old_condition,
> >                                       &mtc->new_condition)) {
> > -            if (ovsdb_condition_is_true(&mtc->new_condition)) {
> > -                if (!ovsdb_condition_is_true(&mtc->old_condition)) {
> > -                    condition->n_true_cnd++;
> > -                }
> > -            } else {
> > -                if (ovsdb_condition_is_true(&mtc->old_condition)) {
> > -                    condition->n_true_cnd--;
> > -                }
> > -            }
> >              ovsdb_condition_destroy(&mtc->old_condition);
> >              ovsdb_condition_clone(&mtc->old_condition, 
> &mtc->new_condition);
> >              ovsdb_monitor_session_condition_set_mode(condition);
> > -- 
> > 2.10.2
> > 
> 
> Thanks for the cleanup. It is a forgotten leftover from a failed effort of 
> "optimization" :)
> 
> Acked-by: Liran Schour <[email protected]>

Thanks Andy and Liran.  I applied this to master.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to