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

Daniel Alvarez Sanchez dalvarez at redhat.com
Wed Feb 28 09:12:32 UTC 2018


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
>>
>
>


More information about the dev mailing list