URL: https://github.com/freeipa/freeipa/pull/488
Author: tiran
Title: #488: Speed up client schema cache
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/488/head:pr488
git checkout pr488
From d3c4393c7c97c6084435a6b072cf9dc553bdf4dd Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Mon, 20 Feb 2017 20:09:13 +0100
Subject: [PATCH 1/2] Speed up client schema cache
It's inefficient to open a zip file over and over again. By loading all
members of the schema cache file at once, the ipa CLI script starts
about 25 to 30% faster for simple cases like help and ping.
Before:
$ time for i in {1..20}; do ./ipa ping >/dev/null; done
real 0m13.608s
user 0m10.316s
sys 0m1.121s
After:
$ time for i in {1..20}; do ./ipa ping >/dev/null; done
real 0m9.330s
user 0m7.635s
sys 0m1.146s
https://fedorahosted.org/freeipa/ticket/6690
Signed-off-by: Christian Heimes <chei...@redhat.com>
---
ipaclient/remote_plugins/schema.py | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 15c03f4..13bdee4 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -458,11 +458,15 @@ def _read_schema(self, fingerprint):
with self._open(fingerprint, 'rb') as f:
self._file.write(f.read())
+ # It's more efficient to read zip file members at once than to open
+ # the zip file a couple of times, see #6690.
with zipfile.ZipFile(self._file, 'r') as schema:
for name in schema.namelist():
ns, _slash, key = name.partition('/')
if ns in self.namespaces:
- self._dict[ns][key] = None
+ self._dict[ns][key] = schema.read(name)
+ elif name == '_help':
+ self._help = schema.read(name)
def __getitem__(self, key):
try:
@@ -520,16 +524,12 @@ def _write_schema(self, fingerprint):
f.truncate(0)
f.write(self._file.read())
- def _read(self, path):
- with zipfile.ZipFile(self._file, 'r') as zf:
- return json.loads(zf.read(path).decode('utf-8'))
-
def read_namespace_member(self, namespace, member):
value = self._dict[namespace][member]
- if value is None:
- path = '{}/{}'.format(namespace, member)
- value = self._dict[namespace][member] = self._read(path)
+ if isinstance(value, bytes):
+ value = json.loads(value.decode('utf-8'))
+ self._dict[namespace][member] = value
return value
@@ -537,8 +537,8 @@ def iter_namespace(self, namespace):
return iter(self._dict[namespace])
def get_help(self, namespace, member):
- if not self._help:
- self._help = self._read('_help')
+ if isinstance(self._help, bytes):
+ self._help = json.loads(self._help.decode('utf-8'))
return self._help[namespace][member]
From 98f596083e1469915f5fd78b9a01164b5beb9d19 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Tue, 28 Feb 2017 10:38:07 +0100
Subject: [PATCH 2/2] Drop in-memory copy of schema zip file
The schema cache used a BytesIO buffer to read/write schema cache before
it got flushed to disk. Since the schema cache is now loaded in one go,
the temporary buffer is no longer needed.
File locking has been replaced with a temporary file and atomic rename.
Signed-off-by: Christian Heimes <chei...@redhat.com>
---
ipaclient/remote_plugins/schema.py | 49 ++++++++++++++------------------------
1 file changed, 18 insertions(+), 31 deletions(-)
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 13bdee4..0cdce9d 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -3,13 +3,11 @@
#
import collections
-import contextlib
import errno
-import fcntl
-import io
import json
import os
import sys
+import tempfile
import types
import zipfile
@@ -374,7 +372,6 @@ def __init__(self, client, fingerprint=None):
self._dict = {}
self._namespaces = {}
self._help = None
- self._file = six.BytesIO()
for ns in self.namespaces:
self._dict[ns] = {}
@@ -404,21 +401,6 @@ def __init__(self, client, fingerprint=None):
self.fingerprint = fingerprint
self.ttl = ttl
- @contextlib.contextmanager
- def _open(self, filename, mode):
- path = os.path.join(self._DIR, filename)
-
- with io.open(path, mode) as f:
- if mode.startswith('r'):
- fcntl.flock(f, fcntl.LOCK_SH)
- else:
- fcntl.flock(f, fcntl.LOCK_EX)
-
- try:
- yield f
- finally:
- fcntl.flock(f, fcntl.LOCK_UN)
-
def _fetch(self, client, ignore_cache=False):
if not client.isconnected():
client.connect(verbose=False)
@@ -454,13 +436,10 @@ def _fetch(self, client, ignore_cache=False):
return (fp, ttl,)
def _read_schema(self, fingerprint):
- self._file.truncate(0)
- with self._open(fingerprint, 'rb') as f:
- self._file.write(f.read())
-
# It's more efficient to read zip file members at once than to open
# the zip file a couple of times, see #6690.
- with zipfile.ZipFile(self._file, 'r') as schema:
+ filename = os.path.join(self._DIR, fingerprint)
+ with zipfile.ZipFile(filename, 'r') as schema:
for name in schema.namelist():
ns, _slash, key = name.partition('/')
if ns in self.namespaces:
@@ -502,8 +481,21 @@ def _write_schema(self, fingerprint):
if e.errno != errno.EEXIST:
raise
- self._file.truncate(0)
- with zipfile.ZipFile(self._file, 'w', zipfile.ZIP_DEFLATED) as schema:
+ with tempfile.NamedTemporaryFile('wb', prefix=fingerprint,
+ dir=self._DIR, delete=False) as f:
+ try:
+ self._write_schema_data(f)
+ f.flush()
+ os.fdatasync(f.fileno())
+ f.close()
+ except Exception:
+ os.unlink(f.name)
+ raise
+ else:
+ os.rename(f.name, os.path.join(self._DIR, fingerprint))
+
+ def _write_schema_data(self, fileobj):
+ with zipfile.ZipFile(fileobj, 'w', zipfile.ZIP_DEFLATED) as schema:
for key, value in self._dict.items():
if key in self.namespaces:
ns = value
@@ -519,11 +511,6 @@ def _write_schema(self, fingerprint):
json.dumps(self._generate_help(self._dict)).encode('utf-8')
)
- self._file.seek(0)
- with self._open(fingerprint, 'wb') as f:
- f.truncate(0)
- f.write(self._file.read())
-
def read_namespace_member(self, namespace, member):
value = self._dict[namespace][member]
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
