0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-17 23:44:39 -05:00

️ Optimized fetching strings from the settings cache

fix https://linear.app/ghost/issue/ENG-1105/settingscacheget-is-slow

- through profiling and flamegraphs, we can see that `_doGet` is one of
  the bottlenecks during high traffic times, sometimes taking up to 20%
  of the CPU time when hammering Ghost with `wrk`
- this is because, for the majority of settings cache lookup, we're
  running `JSON.parse`, which blocks the main thread
- whilst we're only parsing small strings, we're doing it a LOT,
  sometimes hundreds of times per request, which adds up
- this code just throws most deserializing at `JSON.parse`, so if we can
  stop it from doing that, it'd be a huge win
- my initial attempts here were to convert the _doGet function to a
  smarter deserializing, by looking up `cacheEntry.type` and acting
  accordingly
- however, it became a bit of a logical nightmare, and difficult to
  reason about for now (i still think we should do it)
- therefore, I'm just doing to add a hotpath fix to catch 99% of
  usecases, which is checking the type of the cache entry and returning
  the value if it's a string
- on a trivial benchmark locally, this causes Ghost to return 30% more
  requests per second!!
This commit is contained in:
Daniel Lockyer 2024-10-31 09:45:14 +01:00 committed by Daniel Lockyer
parent fa31176621
commit ea6d3a0f26

View file

@ -64,6 +64,10 @@ class CacheManager {
return cacheEntry;
}
// TODO: I think we should be a little smarter here and deserialize the value based on the type
// rather than trying to parse everything as JSON, which is very slow when we do it hundreds
// of times per request.
// Default behavior is to try to resolve the value and return that
try {
// CASE: handle literal false
@ -71,6 +75,11 @@ class CacheManager {
return false;
}
// CASE: hotpath early return for strings which are already strings
if (cacheEntry.type === 'string' && typeof cacheEntry.value === 'string') {
return cacheEntry.value || null;
}
// CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer
if (!isNaN(Number(cacheEntry.value))) {
return cacheEntry.value || null;