settings: use os.fdatasync() and os.fsync() when available

Improve macOS performance by using the `os.fsync()` and `os.fdatasync()`
functions when available.

This completely removes any code path that calls `os.sync()`.

Related-to: #1305
Signed-off-by: David Aguilar <davvid@gmail.com>

cola/core.py

@@ -482,29 +482,43 @@ def _find_executable(executable, path=None):
     return executable
 
 
-def sync(path):
-    """Flush contents to disk. No-op on systems without os.sync() or libc.so.6"""
+def _fdatasync(fd):
+    """fdatasync the file descriptor. Returns True on success"""
     try:
-        libc = ctypes.CDLL('libc.so.6')
-    except OSError:
-        libc = None
-    synced = False
-    has_fdatasync = libc and hasattr(libc, 'fdatasync')
-    has_fsync = libc and hasattr(libc, 'fsync')
-    if has_fdatasync:
-        try:
-            with open(path, 'a', encoding='utf-8') as fd:
-                synced = libc.fdatasync(fd.fileno()) == 0
-        except (OSError, IOError):
-            pass
-    elif has_fsync:
+        os.fdatasync(fd)
+    except (IOError, OSError):
+        pass
+
+
+def _fsync(fd):
+    """fsync the file descriptor. Returns True on success"""
+    try:
+        os.fsync(fd)
+    except (IOError, OSError):
+        pass
+
+
+def fsync(fd):
+    """Flush contents to disk using fdatasync() / fsync()"""
+    has_libc_fdatasync = False
+    has_libc_fsync = False
+    has_os_fdatasync = hasattr(os, 'fdatasync')
+    has_os_fsync = hasattr(os, 'fsync')
+    if not has_os_fdatasync and not has_os_fsync:
         try:
-            with open(path, 'a', encoding='utf-8') as fd:
-                synced = libc.fsync(fd.fileno()) == 0
-        except (OSError, IOError):
-            pass
-    if not synced and hasattr(os, 'sync'):
-        os.sync()
+            libc = ctypes.CDLL('libc.so.6')
+        except OSError:
+            libc = None
+        has_libc_fdatasync = libc and hasattr(libc, 'fdatasync')
+        has_libc_fsync = libc and hasattr(libc, 'fsync')
+    if has_os_fdatasync:
+        _fdatasync(fd)
+    elif has_os_fsync:
+        _fsync(fd)
+    elif has_libc_fdatasync:
+        libc.fdatasync(fd)
+    elif has_libc_fsync:
+        libc.fsync(fd)
 
 
 def rename(old, new):

cola/settings.py

@@ -37,14 +37,16 @@ def read_json(path):
         return {}
 
 
-def write_json(values, path):
+def write_json(values, path, sync=True):
     """Write the specified values dict to a JSON file at the specified path"""
     try:
         parent = os.path.dirname(path)
         if not core.isdir(parent):
             core.makedirs(parent)
         with core.open_write(path) as fp:
             json.dump(values, fp, indent=4)
+            if sync:
+                core.fsync(fp.fileno())
     except (ValueError, TypeError, OSError):
         sys.stderr.write('git-cola: error writing "%s"\n' % path)
         return False
@@ -181,17 +183,14 @@ def save(self, sync=True):
         path_tmp = path + '.tmp'
         path_bak = path + '.bak'
         # Write the new settings to the .tmp file.
-        if not write_json(self.values, path_tmp):
+        if not write_json(self.values, path_tmp, sync=sync):
             return
         # Rename the current settings to a .bak file.
         if core.exists(path) and not rename_path(path, path_bak):
             return
         # Rename the new settings from .tmp to the settings file.
         if not rename_path(path_tmp, path):
             return
-        # Flush the data to disk.
-        if sync:
-            core.sync(path)
         # Delete the .bak file.
         if core.exists(path_bak):
             remove_path(path_bak)

docs/git-cola.rst

@@ -857,7 +857,7 @@ Defaults to `false`.
 
 cola.sync
 ---------
-Set to `false` to disable calling `os.sync()`  / `fdatasync(2)` / `fsync(2)` when saving
+Set to `false` to disable calling `os.fdatasync()`  / `os.fdata()` when saving
 settings. Defaults to `true`, which means that these functions are called when windows
 are closed and their settings are saved.