perf: improve document caching

As per current implementation whenever `get_doc` is called, document is
cached. However, this cache is only ever used by `get_cached_doc`. Going
through the codebase of both FF/ERPNext you'll find that `get_doc` is
used a lot more than `get_cached_doc`. So in many places, all this
caching overhead is unnecessary.

This change removes implicit caching from get_doc and replaces it with
cache-replacement instead. i.e. cache is only updated if it exists but
not created from get_doc.

Pros:
- faster `get_doc`
- lower memory usage on Redis
- Reduces chances of OOM by blowing up worker's memory as old docs can't
  be GCed until
- Correctness i.e. caching only what gets used from cache.

Con:
- After this change. First call to `get_cached_doc` will always be a cache miss. DUH.
This commit is contained in:
Ankush Menat 2022-06-03 15:28:20 +05:30
parent d3b366f147
commit 111df3a8d7
2 changed files with 23 additions and 5 deletions

View file

@ -1023,7 +1023,7 @@ def get_cached_doc(*args, **kwargs):
return doc
if key := can_cache_doc(args):
# local cache
# local cache - has "ready" `Document` objects
if doc := local.document_cache.get(key):
return _respond(doc)
@ -1031,9 +1031,16 @@ def get_cached_doc(*args, **kwargs):
if doc := cache().hget("document_cache", key):
return _respond(doc, True)
# database
# Not found in local/redis, fetch from DB and store in cache
doc = get_doc(*args, **kwargs)
# Store in redis cache
key = get_document_cache_key(doc.doctype, doc.name)
local.document_cache[key] = doc
# Avoid setting in local.cache since there's separate cache
cache().hset("document_cache", key, doc.as_dict(), cache_locally=False)
return doc
@ -1104,10 +1111,12 @@ def get_doc(*args, **kwargs):
doc = frappe.model.document.get_doc(*args, **kwargs)
# set in cache
# Replace cache
if key := can_cache_doc(args):
local.document_cache[key] = doc
cache().hset("document_cache", key, doc.as_dict())
if key in local.document_cache:
local.document_cache[key] = doc
if cache().hexists("document_cache", key):
cache().hset("document_cache", key, doc.as_dict())
return doc

View file

@ -169,6 +169,15 @@ class RedisWrapper(redis.Redis):
except redis.exceptions.ConnectionError:
pass
def hexists(self, name: str, key: str, shared: bool = False) -> bool:
if key is None:
return False
_name = self.make_key(name, shared=shared)
try:
return super(RedisWrapper, self).hexists(_name, key)
except redis.exceptions.ConnectionError:
return False
def hgetall(self, name):
value = super(RedisWrapper, self).hgetall(self.make_key(name))
return {key: pickle.loads(value) for key, value in value.items()}