diff --git a/CHANGELOG.md b/CHANGELOG.md index 423144c..2ae1bda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ diskcache-community forked from DiskCache at upstream commit ebfa37c (3 March 20 ### Performance - `Cache.get` is about 15% faster on a hit and 12% on a miss, and `DjangoCache.get` about 16%: a plain lookup reads only the value columns, on a cursor reused per thread (PR #38). +- `Cache.set` is about 15% faster, and `DjangoCache.set` about 17%: each write runs fewer SQL statements and less Python (PR #37). ### Packaging diff --git a/diskcache_community/core.py b/diskcache_community/core.py index ea62218..8647070 100644 --- a/diskcache_community/core.py +++ b/diskcache_community/core.py @@ -1618,6 +1618,9 @@ class _ReaderMissing(ImportError): # Connections inherited from a parent process, kept open on purpose. _INHERITED_CONNECTIONS: 'list[sqlite3.Connection]' = [] +# SQLite 3.16 added table-valued pragma functions such as pragma_page_count(). +_PRAGMA_FUNCTIONS = sqlite3.sqlite_version_info >= (3, 16, 0) + def _before_fork(): # SQLite advises closing every connection before fork. The gate waits @@ -2209,20 +2212,37 @@ def set(self, key, value, expire=None, read=False, tag=None, retry=False): # need cleanup. with self._transact(retry, filename) as (sql, cleanup): - rows = sql( - 'SELECT rowid, filename FROM Cache' - ' WHERE key = ? AND raw = ?', + # One statement finds the row for `key`, if any, and the earliest + # expire time in the cache, so that culling can skip its search + # for expired items when there are none. + ((earliest, rowid, old_filename),) = sql( + 'SELECT' + ' (SELECT min(expire_time) FROM Cache' + ' WHERE expire_time IS NOT NULL),' + ' Cache.rowid, Cache.filename' + ' FROM (SELECT 1) LEFT JOIN Cache' + ' ON Cache.key = ? AND Cache.raw = ?', (db_key, raw), ).fetchall() - if rows: - ((rowid, old_filename),) = rows + if rowid is not None: cleanup(old_filename) self._row_update(rowid, now, columns) else: self._row_insert(db_key, raw, now, columns) - self._cull(now, sql, cleanup) + # The row just written may itself have expired already. + if expire_time is not None and ( + earliest is None or expire_time < earliest + ): + earliest = expire_time + + self._cull( + now, + sql, + cleanup, + expired=earliest is not None and earliest < now, + ) return True @@ -2289,7 +2309,9 @@ def _row_insert(self, key, raw, now, columns): ), ) - def _cull(self, now, sql, cleanup, limit=None): + def _cull(self, now, sql, cleanup, limit=None, expired=True): + # `expired` is False when the caller knows that no item has expired, + # which skips the search for expired items. cull_limit = self.cull_limit if limit is None else limit if cull_limit == 0: @@ -2303,8 +2325,11 @@ def _cull(self, now, sql, cleanup, limit=None): ' ORDER BY expire_time LIMIT ?' ) - select_expired = select_expired_template % 'filename' - rows = sql(select_expired, (now, cull_limit)).fetchall() + if expired: + select_expired = select_expired_template % 'filename' + rows = sql(select_expired, (now, cull_limit)).fetchall() + else: + rows = [] if rows: delete_expired = 'DELETE FROM Cache WHERE rowid IN (%s)' % ( @@ -3986,6 +4011,18 @@ def volume(self): :return: size in bytes """ + if _PRAGMA_FUNCTIONS: + # One statement instead of two: set calls this on every write + # when an eviction policy is in effect. + rows = self._sql_retry( + 'SELECT (SELECT page_count FROM pragma_page_count()), value' + " FROM Settings WHERE key = 'size'" + ).fetchall() + if rows: + ((page_count, size),) = rows + setattr(self, 'size', size) # As reset('size') does. + return self._page_size * page_count + size + ((page_count,),) = self._sql('PRAGMA page_count').fetchall() total_size = self._page_size * page_count + self.reset('size') return total_size diff --git a/tests/test_core.py b/tests/test_core.py index 79dbd9e..77b9b70 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -480,6 +480,43 @@ def test_expire_rows(cache): assert len(cache.check()) == 0 +def test_set_culls_rows_expired_by_another_connection(cache): + for value in range(5): + assert cache.set(value, value, expire=60) + + with dc.Cache(cache.directory) as other: + other._sql('UPDATE Cache SET expire_time = ?', (time.time() - 1,)) + + assert cache.set(5, 5) + assert len(cache) == 1 + assert len(cache.check()) == 0 + + +def test_set_culls_its_own_expired_row(cache): + assert cache.set(0, 0) + assert cache.set(1, 1, expire=-1) + assert len(cache) == 1 + assert cache.get(0) == 0 + + +def test_set_without_expired_rows_skips_search(cache): + assert cache.set(0, 0, expire=60) + statements = [] + cache._con.set_trace_callback(statements.append) + try: + assert cache.set(1, 1) + finally: + cache._con.set_trace_callback(None) + assert not any('expire_time < ' in text for text in statements) + + +def test_volume_without_pragma_functions(cache): + cache.set(0, b'x' * 1000) + expected = cache.volume() + with mock.patch.object(dc.core, '_PRAGMA_FUNCTIONS', False): + assert cache.volume() == expected + + def test_least_recently_stored(cache): cache.reset('eviction_policy', 'least-recently-stored') cache.reset('size_limit', int(10.1e6)) diff --git a/tests/test_readonly.py b/tests/test_readonly.py index f4fede2..da20886 100644 --- a/tests/test_readonly.py +++ b/tests/test_readonly.py @@ -386,6 +386,16 @@ def test_read_only_django(directory): cache.close() +def test_read_only_volume_without_size_row(directory): + with dc.Cache(directory) as cache: + cache['a'] = 1 + cache._sql("DELETE FROM Settings WHERE key = 'size'") + + with dc.Cache(directory, read_only=True) as cache: + assert cache.volume() > 0 + assert cache.size == 0 + + def test_read_only_without_metadata_rows(directory): with dc.Cache(directory) as cache: cache['a'] = 1