Re: [ovs-dev] [PATCH v2] python: idl: Avoid pre-allocating column defaults

2021-11-29 Thread Ilya Maximets
On 11/5/21 15:13, Terry Wilson wrote:
> Many python implementations pre-allocate space for multiple
> objects in empty dicts and lists. Using a custom dict-like object
> that only generates these objects when they are accessed can save
> memory.
> 
> On a fairly pathological case where the DB has 1000 networks each
> with 100 ports, with only 'name' fields set, this saves around
> 300MB of memory.
> 
> One could argue that if values are not going to change from their
> defaults, then users should not be monitoring those columns, but
> it's also probably good to not waste memory even if user code is
> sub-optimal.
> 
> Signed-off-by: Terry Wilson 
> ---
>  python/ovs/db/idl.py | 39 +--
>  1 file changed, 33 insertions(+), 6 deletions(-)

Thanks, Terry, Dumitru and Flavio!  Applied.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] python: idl: Avoid pre-allocating column defaults

2021-11-23 Thread Flavio Fernandes
LGTM!

Acked-by: Flavio Fernandes 




> -- Forwarded message -
> From: Terry Wilson mailto:twil...@redhat.com>>
> Date: Fri, Nov 5, 2021 at 10:14 AM
> Subject: [ovs-dev] [PATCH v2] python: idl: Avoid pre-allocating column 
> defaults
> To: mailto:d...@openvswitch.org>>
> 
> 
> Many python implementations pre-allocate space for multiple
> objects in empty dicts and lists. Using a custom dict-like object
> that only generates these objects when they are accessed can save
> memory.
> 
> On a fairly pathological case where the DB has 1000 networks each
> with 100 ports, with only 'name' fields set, this saves around
> 300MB of memory.
> 
> One could argue that if values are not going to change from their
> defaults, then users should not be monitoring those columns, but
> it's also probably good to not waste memory even if user code is
> sub-optimal.
> 
> Signed-off-by: Terry Wilson mailto:twil...@redhat.com>>
> ---
>  python/ovs/db/idl.py | 39 +--
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index ecae5e143..402aadabc 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -45,6 +45,36 @@ Notice = collections.namedtuple('Notice', ('event', 'row', 
> 'updates'))
>  Notice.__new__.__defaults__ = (None,)  # default updates=None
> 
> 
> +class ColumnDefaultDict(dict):
> +"""A column dictionary with on-demand generated default values
> +
> +This object acts like the Row._data column dictionary, but without the
> +necessity of populating column default values. These values are generated
> +on-demand and therefore only use memory once they are accessed.
> +"""
> +__slots__ = ('_table', )
> +
> +def __init__(self, table):
> +self._table = table
> +super().__init__()
> +
> +def __missing__(self, column):
> +column = self._table.columns[column]
> +return ovs.db.data.Datum.default(column.type)
> +
> +def keys(self):
> +return self._table.columns.keys()
> +
> +def values(self):
> +return iter(self[k] for k in self)
> +
> +def __iter__(self):
> +return iter(self.keys())
> +
> +def __contains__(self, item):
> +return item in self.keys()
> +
> +
>  class Idl(object):
>  """Open vSwitch Database Interface Definition Language (OVSDB IDL).
> 
> @@ -908,10 +938,7 @@ class Idl(object):
>  return changed
> 
>  def __create_row(self, table, uuid):
> -data = {}
> -for column in table.columns.values():
> -data[column.name <http://column.name/>] = 
> ovs.db.data.Datum.default(column.type)
> -return Row(self, table, uuid, data)
> +return Row(self, table, uuid, ColumnDefaultDict(table))
> 
>  def __error(self):
>  self._session.force_reconnect()
> @@ -1249,7 +1276,7 @@ class Row(object):
>  A transaction must be in progress."""
>  assert self._idl.txn
>  assert self._changes is not None
> -if not self._data or column_name in self._changes:
> +if self._data is None or column_name in self._changes:
>  return
> 
>  self._prereqs[column_name] = None
> @@ -1777,7 +1804,7 @@ class Transaction(object):
>  # transaction only does writes of existing values, without making any
>  # real changes, we will drop the whole transaction later in
>  # ovsdb_idl_txn_commit().)
> -if (not column.alert and row._data and
> +if (not column.alert and row._data is not None and
>  row._data.get(column.name <http://column.name/>) == datum):
>  new_value = row._changes.get(column.name <http://column.name/>)
>  if new_value is None or new_value == datum:
> -- 
> 2.31.1
> 
> ___
> dev mailing list
> d...@openvswitch.org <mailto:d...@openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev 
> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] python: idl: Avoid pre-allocating column defaults

2021-11-05 Thread Dumitru Ceara
On 11/5/21 3:13 PM, Terry Wilson wrote:
> Many python implementations pre-allocate space for multiple
> objects in empty dicts and lists. Using a custom dict-like object
> that only generates these objects when they are accessed can save
> memory.
> 
> On a fairly pathological case where the DB has 1000 networks each
> with 100 ports, with only 'name' fields set, this saves around
> 300MB of memory.
> 
> One could argue that if values are not going to change from their
> defaults, then users should not be monitoring those columns, but
> it's also probably good to not waste memory even if user code is
> sub-optimal.
> 
> Signed-off-by: Terry Wilson 
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2] python: idl: Avoid pre-allocating column defaults

2021-11-05 Thread Terry Wilson
Many python implementations pre-allocate space for multiple
objects in empty dicts and lists. Using a custom dict-like object
that only generates these objects when they are accessed can save
memory.

On a fairly pathological case where the DB has 1000 networks each
with 100 ports, with only 'name' fields set, this saves around
300MB of memory.

One could argue that if values are not going to change from their
defaults, then users should not be monitoring those columns, but
it's also probably good to not waste memory even if user code is
sub-optimal.

Signed-off-by: Terry Wilson 
---
 python/ovs/db/idl.py | 39 +--
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index ecae5e143..402aadabc 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -45,6 +45,36 @@ Notice = collections.namedtuple('Notice', ('event', 'row', 
'updates'))
 Notice.__new__.__defaults__ = (None,)  # default updates=None
 
 
+class ColumnDefaultDict(dict):
+"""A column dictionary with on-demand generated default values
+
+This object acts like the Row._data column dictionary, but without the
+necessity of populating column default values. These values are generated
+on-demand and therefore only use memory once they are accessed.
+"""
+__slots__ = ('_table', )
+
+def __init__(self, table):
+self._table = table
+super().__init__()
+
+def __missing__(self, column):
+column = self._table.columns[column]
+return ovs.db.data.Datum.default(column.type)
+
+def keys(self):
+return self._table.columns.keys()
+
+def values(self):
+return iter(self[k] for k in self)
+
+def __iter__(self):
+return iter(self.keys())
+
+def __contains__(self, item):
+return item in self.keys()
+
+
 class Idl(object):
 """Open vSwitch Database Interface Definition Language (OVSDB IDL).
 
@@ -908,10 +938,7 @@ class Idl(object):
 return changed
 
 def __create_row(self, table, uuid):
-data = {}
-for column in table.columns.values():
-data[column.name] = ovs.db.data.Datum.default(column.type)
-return Row(self, table, uuid, data)
+return Row(self, table, uuid, ColumnDefaultDict(table))
 
 def __error(self):
 self._session.force_reconnect()
@@ -1249,7 +1276,7 @@ class Row(object):
 A transaction must be in progress."""
 assert self._idl.txn
 assert self._changes is not None
-if not self._data or column_name in self._changes:
+if self._data is None or column_name in self._changes:
 return
 
 self._prereqs[column_name] = None
@@ -1777,7 +1804,7 @@ class Transaction(object):
 # transaction only does writes of existing values, without making any
 # real changes, we will drop the whole transaction later in
 # ovsdb_idl_txn_commit().)
-if (not column.alert and row._data and
+if (not column.alert and row._data is not None and
 row._data.get(column.name) == datum):
 new_value = row._changes.get(column.name)
 if new_value is None or new_value == datum:
-- 
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev