From 439d4c1061567280f4be1ebe61ab57d7df50b521 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Wed, 28 Nov 2018 14:06:27 +0100 Subject: [PATCH 1/2] Performance improvement: Type cast properties on demand --- beets/dbcore/db.py | 32 ++++++++++++++++++++++++++++---- beets/importer.py | 8 ++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 24c20ef1a..112b8ee93 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -177,6 +177,8 @@ class Model(object): """ self._db = db self._dirty = set() + self._raw_values_fixed = {} + self._raw_values_flex = {} self._values_fixed = {} self._values_flex = {} @@ -193,9 +195,9 @@ class Model(object): """ obj = cls(db) for key, value in fixed_values.items(): - obj._values_fixed[key] = cls._type(key).from_sql(value) + obj._raw_values_fixed[key] = value for key, value in flex_values.items(): - obj._values_flex[key] = cls._type(key).from_sql(value) + obj._raw_values_flex[key] = value return obj def __repr__(self): @@ -232,7 +234,9 @@ class Model(object): """ new = self.__class__() new._db = self._db + new._raw_values_fixed = self._raw_values_fixed.copy() new._values_fixed = self._values_fixed.copy() + new._raw_values_flex = self._raw_values_flex.copy() new._values_flex = self._values_flex.copy() new._dirty = self._dirty.copy() return new @@ -256,9 +260,18 @@ class Model(object): if key in getters: # Computed. return getters[key](self) elif key in self._fields: # Fixed. - return self._values_fixed.get(key, self._type(key).null) + if key in self._values_fixed: + return self._values_fixed[key] + elif key in self._raw_values_fixed: + self._values_fixed[key] = self._type(key).from_sql(self._raw_values_fixed[key]) + return self._values_fixed[key] + else: + return self._type(key).null elif key in self._values_flex: # Flexible. return self._values_flex[key] + elif key in self._raw_values_flex: # Flexible. + self._values_flex[key] = self._type(key).from_sql(self._raw_values_flex[key]) + return self._values_flex[key] else: raise KeyError(key) @@ -268,8 +281,12 @@ class Model(object): """ # Choose where to place the value. if key in self._fields: + if not key in self._values_fixed and key in self._raw_values_fixed: + self._values_fixed[key] = self._type(key).from_sql(self._raw_values_fixed[key]) source = self._values_fixed else: + if not key in self._values_flex and key in self._raw_values_flex: + self._values_flex[key] = self._type(key).from_sql(self._raw_values_flex[key]) source = self._values_flex # If the field has a type, filter the value. @@ -294,6 +311,11 @@ class Model(object): """ if key in self._values_flex: # Flexible. del self._values_flex[key] + if key in self._raw_values_flex: + del self._raw_values_flex[key] + self._dirty.add(key) # Mark for dropping on store. + elif key in self._raw_values_flex: # Flexible + del self._raw_values_flex[key] self._dirty.add(key) # Mark for dropping on store. elif key in self._fields: # Fixed setattr(self, key, self._type(key).null) @@ -307,7 +329,7 @@ class Model(object): `computed` parameter controls whether computed (plugin-provided) fields are included in the key list. """ - base_keys = list(self._fields) + list(self._values_flex.keys()) + base_keys = list(self._fields) + list(self._values_flex.keys()) + list(self._raw_values_flex.keys()) if computed: return base_keys + list(self._getters().keys()) else: @@ -436,7 +458,9 @@ class Model(object): self._check_db() stored_obj = self._db._get(type(self), self.id) assert stored_obj is not None, u"object {0} not in DB".format(self.id) + self._raw_values_fixed = {} self._values_fixed = {} + self._raw_values_flex = {} self._values_flex = {} self.update(dict(stored_obj)) self.clear_dirty() diff --git a/beets/importer.py b/beets/importer.py index 889f1297e..50e4545c5 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -785,14 +785,14 @@ class ImportTask(BaseImportTask): replaced_album = self.replaced_albums.get(self.album.path) if replaced_album: self.album.added = replaced_album.added - self.album.update(replaced_album._values_flex) + self.album.update(replaced_album._raw_values_flex) self.album.artpath = replaced_album.artpath self.album.store() log.debug( u'Reimported album: added {0}, flexible ' u'attributes {1} from album {2} for {3}', self.album.added, - replaced_album._values_flex.keys(), + replaced_album._raw_values_flex.keys(), replaced_album.id, displayable_path(self.album.path) ) @@ -809,11 +809,11 @@ class ImportTask(BaseImportTask): dup_item.id, displayable_path(item.path) ) - item.update(dup_item._values_flex) + item.update(dup_item._raw_values_flex) log.debug( u'Reimported item flexible attributes {0} ' u'from item {1} for {2}', - dup_item._values_flex.keys(), + dup_item._raw_values_flex.keys(), dup_item.id, displayable_path(item.path) ) From f9f2bddba0b97adc7abfdb9da59362a78c92ddf7 Mon Sep 17 00:00:00 2001 From: Heinz Wiesinger Date: Sun, 24 Mar 2019 17:40:33 +0100 Subject: [PATCH 2/2] Add class wrapper for lazily converting attributes --- beets/dbcore/db.py | 133 +++++++++++++++++++++++++++++++++++---------- beets/importer.py | 8 +-- 2 files changed, 107 insertions(+), 34 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 112b8ee93..b8243e9df 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -88,6 +88,100 @@ class FormattedMapping(collections.Mapping): return value +class LazyConvertDict(object): + """Lazily convert types for attributes fetched from the database + """ + + def __init__(self, model_cls): + """Initialize the object empty + """ + self.data = {} + self.model_cls = model_cls + self._converted = {} + + def init(self, data): + """Set the base data that should be lazily converted + """ + self.data = data + + def _convert(self, key, value): + """Convert the attribute type according the the SQL type + """ + return self.model_cls._type(key).from_sql(value) + + def __setitem__(self, key, value): + """Set an attribute value, assume it's already converted + """ + self._converted[key] = value + + def __getitem__(self, key): + """Get an attribute value, converting the type on demand + if needed + """ + if key in self._converted: + return self._converted[key] + elif key in self.data: + value = self._convert(key, self.data[key]) + self._converted[key] = value + return value + + def __delitem__(self, key): + """Delete both converted and base data + """ + if key in self._converted: + del self._converted[key] + if key in self.data: + del self.data[key] + + def keys(self): + """Get a list of available field names for this object. + """ + return list(self._converted.keys()) + list(self.data.keys()) + + def copy(self): + """Create a copy of the object. + """ + new = self.__class__(self.model_cls) + new.data = self.data.copy() + new._converted = self._converted.copy() + return new + + # Act like a dictionary. + + def update(self, values): + """Assign all values in the given dict. + """ + for key, value in values.items(): + self[key] = value + + def items(self): + """Iterate over (key, value) pairs that this object contains. + Computed fields are not included. + """ + for key in self: + yield key, self[key] + + def get(self, key, default=None): + """Get the value for a given key or `default` if it does not + exist. + """ + if key in self: + return self[key] + else: + return default + + def __contains__(self, key): + """Determine whether `key` is an attribute on this object. + """ + return key in self.keys() + + def __iter__(self): + """Iterate over the available field names (excluding computed + fields). + """ + return iter(self.keys()) + + # Abstract base for model classes. class Model(object): @@ -177,10 +271,8 @@ class Model(object): """ self._db = db self._dirty = set() - self._raw_values_fixed = {} - self._raw_values_flex = {} - self._values_fixed = {} - self._values_flex = {} + self._values_fixed = LazyConvertDict(self) + self._values_flex = LazyConvertDict(self) # Initial contents. self.update(values) @@ -194,10 +286,10 @@ class Model(object): ordinary construction are bypassed. """ obj = cls(db) - for key, value in fixed_values.items(): - obj._raw_values_fixed[key] = value - for key, value in flex_values.items(): - obj._raw_values_flex[key] = value + + obj._values_fixed.init(fixed_values) + obj._values_flex.init(flex_values) + return obj def __repr__(self): @@ -234,9 +326,7 @@ class Model(object): """ new = self.__class__() new._db = self._db - new._raw_values_fixed = self._raw_values_fixed.copy() new._values_fixed = self._values_fixed.copy() - new._raw_values_flex = self._raw_values_flex.copy() new._values_flex = self._values_flex.copy() new._dirty = self._dirty.copy() return new @@ -262,16 +352,10 @@ class Model(object): elif key in self._fields: # Fixed. if key in self._values_fixed: return self._values_fixed[key] - elif key in self._raw_values_fixed: - self._values_fixed[key] = self._type(key).from_sql(self._raw_values_fixed[key]) - return self._values_fixed[key] else: return self._type(key).null elif key in self._values_flex: # Flexible. return self._values_flex[key] - elif key in self._raw_values_flex: # Flexible. - self._values_flex[key] = self._type(key).from_sql(self._raw_values_flex[key]) - return self._values_flex[key] else: raise KeyError(key) @@ -281,12 +365,8 @@ class Model(object): """ # Choose where to place the value. if key in self._fields: - if not key in self._values_fixed and key in self._raw_values_fixed: - self._values_fixed[key] = self._type(key).from_sql(self._raw_values_fixed[key]) source = self._values_fixed else: - if not key in self._values_flex and key in self._raw_values_flex: - self._values_flex[key] = self._type(key).from_sql(self._raw_values_flex[key]) source = self._values_flex # If the field has a type, filter the value. @@ -311,11 +391,6 @@ class Model(object): """ if key in self._values_flex: # Flexible. del self._values_flex[key] - if key in self._raw_values_flex: - del self._raw_values_flex[key] - self._dirty.add(key) # Mark for dropping on store. - elif key in self._raw_values_flex: # Flexible - del self._raw_values_flex[key] self._dirty.add(key) # Mark for dropping on store. elif key in self._fields: # Fixed setattr(self, key, self._type(key).null) @@ -329,7 +404,7 @@ class Model(object): `computed` parameter controls whether computed (plugin-provided) fields are included in the key list. """ - base_keys = list(self._fields) + list(self._values_flex.keys()) + list(self._raw_values_flex.keys()) + base_keys = list(self._fields) + list(self._values_flex.keys()) if computed: return base_keys + list(self._getters().keys()) else: @@ -458,10 +533,8 @@ class Model(object): self._check_db() stored_obj = self._db._get(type(self), self.id) assert stored_obj is not None, u"object {0} not in DB".format(self.id) - self._raw_values_fixed = {} - self._values_fixed = {} - self._raw_values_flex = {} - self._values_flex = {} + self._values_fixed = LazyConvertDict(self) + self._values_flex = LazyConvertDict(self) self.update(dict(stored_obj)) self.clear_dirty() diff --git a/beets/importer.py b/beets/importer.py index 50e4545c5..889f1297e 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -785,14 +785,14 @@ class ImportTask(BaseImportTask): replaced_album = self.replaced_albums.get(self.album.path) if replaced_album: self.album.added = replaced_album.added - self.album.update(replaced_album._raw_values_flex) + self.album.update(replaced_album._values_flex) self.album.artpath = replaced_album.artpath self.album.store() log.debug( u'Reimported album: added {0}, flexible ' u'attributes {1} from album {2} for {3}', self.album.added, - replaced_album._raw_values_flex.keys(), + replaced_album._values_flex.keys(), replaced_album.id, displayable_path(self.album.path) ) @@ -809,11 +809,11 @@ class ImportTask(BaseImportTask): dup_item.id, displayable_path(item.path) ) - item.update(dup_item._raw_values_flex) + item.update(dup_item._values_flex) log.debug( u'Reimported item flexible attributes {0} ' u'from item {1} for {2}', - dup_item._raw_values_flex.keys(), + dup_item._values_flex.keys(), dup_item.id, displayable_path(item.path) )