[Sugar-devel] [PATCH sugar-toolkit v2] sugar.datastore.datastore: Avoid data update if the file hasn't been changed

[Sascha Silbe]

If the path hasn't been changed since the file was checked out and device  /
inode numbers still match, the contents are identical to the data store entry
(because it's a link), so we can do a metadata-only update.

This optimises the case where an activity (e.g. Record) reads the data, but
changes only the metadata.

Reported-By: Daniel Drake <dsd at laptop.org>
Signed-off-by: Sascha Silbe <silbe at activitycentral.com>
---
v1->v2: cope with file_path being explicitly set to '' instead of None (like
        sugar.activity.activity.Activity._initialize_journal_object() does)

 src/sugar/datastore/datastore.py |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/sugar/datastore/datastore.py b/src/sugar/datastore/datastore.py
index ed05634..11223b0 100644
--- a/src/sugar/datastore/datastore.py
+++ b/src/sugar/datastore/datastore.py
@@ -148,6 +148,7 @@ class DSObject(object):
         self._file_path = file_path
         self._destroyed = False
         self._owns_file = False
+        self._checkout_inode_info = None

     def get_object_id(self):
         return self._object_id
@@ -181,19 +182,32 @@ class DSObject(object):

     metadata = property(get_metadata, set_metadata)

-    def get_file_path(self, fetch=True):
+    def get_file_path(self, fetch=True, none_if_unchanged=False):
         if fetch and self._file_path is None and not self.object_id is None:
             self.set_file_path(_get_data_store().get_filename(self.object_id))
             self._owns_file = True
+            stat = os.stat(self._file_path)
+            self._checkout_inode_info = (stat.st_dev, stat.st_ino)
+
+        if none_if_unchanged and self._file_path:
+            stat = os.stat(self._file_path)
+            if self._checkout_inode_info == (stat.st_dev, stat.st_ino):
+                # still identical to the data store checkout (hard link)
+                # => no data update required
+                return None
+
         return self._file_path

     def set_file_path(self, file_path):
-        if self._file_path != file_path:
-            if self._file_path and self._owns_file:
-                if os.path.isfile(self._file_path):
-                    os.remove(self._file_path)
-                self._owns_file = False
-            self._file_path = file_path
+        if self._file_path == file_path:
+            return
+
+        self._checkout_inode_info = None
+        if self._file_path and self._owns_file:
+            if os.path.isfile(self._file_path):
+                os.remove(self._file_path)
+            self._owns_file = False
+        self._file_path = file_path

     file_path = property(get_file_path, set_file_path)

@@ -363,7 +377,7 @@ def write(ds_object, update_mtime=True, transfer_ownership=False,
         properties['mtime'] = datetime.now().isoformat()
         properties['timestamp'] = int(time.time())

-    file_path = ds_object.get_file_path(fetch=False)
+    file_path = ds_object.get_file_path(fetch=False, none_if_unchanged=True)
     if file_path is None:
         file_path = ''

--
1.7.2.3