[ovs-dev] [PATCH] OVN python IDL: avoid useless JSON conversion to enhance performance

Miguel Angel Ajo Pelayo majopela at redhat.com
Wed Jun 13 15:47:31 UTC 2018


Thank you very much Ben :)

On Wed, Jun 13, 2018 at 5:20 PM Ben Pfaff <blp at ovn.org> wrote:

> OK, I crossported to branch-2.9.
>
> On Wed, Jun 13, 2018 at 01:24:44PM +0200, Miguel Angel Ajo Pelayo wrote:
> > Ben, Russell, could we get this down to ovs 2.9?
> >
> > It's very important for scale.
> >
> > Best regards,
> > Miguel Ángel.
> >
> > On Wed, Jun 13, 2018 at 1:13 PM Daniel Alvarez Sanchez <
> dalvarez at redhat.com>
> > wrote:
> >
> > > Hi,
> > > Bringing this up again.
> > > Can we please get this backported into 2.9 branch? The enhancement is
> very
> > > noticeable.
> > > Thanks a lot!
> > > Daniel
> > >
> > > On Tue, Mar 6, 2018 at 5:09 PM, Lucas Alvares Gomes <
> lucasagomes at gmail.com
> > > > wrote:
> > >
> > >> Hi,
> > >>
> > >> Excellent investigative work here Daniel, thanks for that!
> > >>
> > >> On Wed, Feb 28, 2018 at 9:37 AM, Miguel Angel Ajo Pelayo
> > >> <majopela at redhat.com> wrote:
> > >> > And if we can get backports down the lane, that'd be great :)
> > >> >
> > >>
> > >> +1 for backporting it. The changes seems straight forward to do so.
> > >>
> > >> > On Wed, Feb 28, 2018 at 9:37 AM Miguel Angel Ajo Pelayo <
> > >> majopela at redhat.com>
> > >> > wrote:
> > >> >
> > >> >> Acked-by: Miguel Angel Ajo <majopela at redhat.com>
> > >> >>
> > >> >> On Wed, Feb 28, 2018 at 9:13 AM Daniel Alvarez Sanchez <
> > >> >> dalvarez at redhat.com> wrote:
> > >> >>
> > >> >>> Thanks Terry and Han for the reviews!
> > >> >>> I removed the 'OVN' keyword from the title and submitted a v2:
> > >> >>> [PATCH v2] python: avoid useless JSON conversion to enhance
> > >> performance
> > >> >>>
> > >> >>> Thanks again.
> > >> >>> Daniel
> > >> >>>
> > >> >>> On Wed, Feb 28, 2018 at 12:41 AM, Han Zhou <zhouhan at gmail.com>
> wrote:
> > >> >>>
> > >> >>> > Great catch!
> > >> >>> > This patch is generic and not only for OVN, so I suggest to
> remove
> > >> the
> > >> >>> > "OVN" keyword in commit title.
> > >> >>> >
> > >> >>> > Acked-by: Han Zhou <hzhou8 at ebay.com>
> > >> >>> >
> > >> >>> > On Tue, Feb 27, 2018 at 12:44 PM, Terry Wilson <
> twilson at redhat.com>
> > >> >>> wrote:
> > >> >>> >
> > >> >>> >> On Tue, Feb 27, 2018 at 9:25 AM, Daniel Alvarez <
> > >> dalvarez at redhat.com>
> > >> >>> >> wrote:
> > >> >>> >> > This patch removes a useless conversion to/from JSON in the
> > >> >>> >> > processing of any 'modify' operations inside the
> process_update2
> > >> >>> >> > method in Python IDL implementation.
> > >> >>> >> >
> > >> >>> >> > Previous code will make resources creation take longer as the
> > >> number
> > >> >>> >> > of elements in the row grows because of that JSON conversion.
> > >> This
> > >> >>> >> > patch eliminates it and now the time remains consant
> regardless
> > >> >>> >> > of the database contents improving performance and scaling.
> > >> >>> >> >
> > >> >>> >> > Reported-by: Daniel Alvarez Sanchez <dalvarez at redhat.com>
> > >> >>> >> > Reported-at: https://mail.openvswitch.org/p
> > >> >>> >> ipermail/ovs-discuss/2018-February/046263.html
> > >> >>> >> > Signed-off-by: Daniel Alvarez <dalvarez at redhat.com>
> > >> >>> >> > ---
> > >> >>> >> >  python/ovs/db/idl.py | 12 +++++-------
> > >> >>> >> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > >> >>> >> >
> > >> >>> >> > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> > >> >>> >> > index 60548bcf5..5a4d129c0 100644
> > >> >>> >> > --- a/python/ovs/db/idl.py
> > >> >>> >> > +++ b/python/ovs/db/idl.py
> > >> >>> >> > @@ -518,10 +518,8 @@ class Idl(object):
> > >> >>> >> >              if not row:
> > >> >>> >> >                  raise error.Error('Modify non-existing row')
> > >> >>> >> >
> > >> >>> >> > -            old_row_diff_json = self.__apply_diff(table,
> row,
> > >> >>> >> > -
> > >> >>> row_update['modify'])
> > >> >>> >> > -            self.notify(ROW_UPDATE, row,
> > >> >>> >> > -                        Row.from_json(self, table, uuid,
> > >> >>> >> old_row_diff_json))
> > >> >>> >> > +            old_row = self.__apply_diff(table, row,
> > >> >>> >> row_update['modify'])
> > >> >>> >> > +            self.notify(ROW_UPDATE, row, Row(self, table,
> uuid,
> > >> >>> >> old_row))
> > >> >>> >> >              changed = True
> > >> >>> >> >          else:
> > >> >>> >> >              raise error.Error('<row-update> unknown
> operation',
> > >> >>> >> > @@ -584,7 +582,7 @@ class Idl(object):
> > >> >>> >> >                          row_update[column.name] =
> > >> >>> >> self.__column_name(column)
> > >> >>> >> >
> > >> >>> >> >      def __apply_diff(self, table, row, row_diff):
> > >> >>> >> > -        old_row_diff_json = {}
> > >> >>> >> > +        old_row = {}
> > >> >>> >> >          for column_name, datum_diff_json in
> > >> six.iteritems(row_diff):
> > >> >>> >> >              column = table.columns.get(column_name)
> > >> >>> >> >              if not column:
> > >> >>> >> > @@ -601,12 +599,12 @@ class Idl(object):
> > >> >>> >> >                            % (column_name, table.name, e))
> > >> >>> >> >                  continue
> > >> >>> >> >
> > >> >>> >> > -            old_row_diff_json[column_name] =
> > >> >>> >> row._data[column_name].to_json()
> > >> >>> >> > +            old_row[column_name] =
> row._data[column_name].copy()
> > >> >>> >> >              datum = row._data[column_name].diff(datum_diff)
> > >> >>> >> >              if datum != row._data[column_name]:
> > >> >>> >> >                  row._data[column_name] = datum
> > >> >>> >> >
> > >> >>> >> > -        return old_row_diff_json
> > >> >>> >> > +        return old_row
> > >> >>> >> >
> > >> >>> >> >      def __row_update(self, table, row, row_json):
> > >> >>> >> >          changed = False
> > >> >>> >> > --
> > >> >>> >> > 2.13.5
> > >> >>> >> >
> > >> >>> >> > _______________________________________________
> > >> >>> >> > dev mailing list
> > >> >>> >> > dev at openvswitch.org
> > >> >>> >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >> >>> >>
> > >> >>> >> +1
> > >> >>> >>
> > >> >>> >> This passes all of the functional tests in ovsdbapp when
> applied.
> > >> Nice
> > >> >>> >> find!
> > >> >>> >>
> > >> >>> >> Terry
> > >> >>> >> _______________________________________________
> > >> >>> >> dev mailing list
> > >> >>> >> dev at openvswitch.org
> > >> >>> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >> >>> >>
> > >> >>> >
> > >> >>> >
> > >> >>> _______________________________________________
> > >> >>> dev mailing list
> > >> >>> dev at openvswitch.org
> > >> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >> >>>
> > >> >>
> > >> > _______________________________________________
> > >> > dev mailing list
> > >> > dev at openvswitch.org
> > >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >>
> > >
> > >
>


More information about the dev mailing list