perf: Memoize the common_bootstrap_payload and include user param (#21018) #21439
Conversation
Try patch Co-authored-by: Bogdan Kyryliuk <bogdankyryliuk@dropbox.com>
Codecov Report
@@ Coverage Diff @@
## master #21439 +/- ##
===========================================
- Coverage 66.53% 55.26% -11.28%
===========================================
Files 1791 1791
Lines 68599 68602 +3
Branches 7320 7320
===========================================
- Hits 45645 37911 -7734
- Misses 21064 28801 +7737
Partials 1890 1890
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
bkyryliuk
force-pushed
the
bogdan/cache_bootstrap_try2
branch
from
ce2d965
to
608ba48
Compare
bkyryliuk
changed the title
There was a problem hiding this comment.
LGTM and tested to work as expected. I'd love to see all instances of @superset.utils.memoized.memoized replaced with @cache_manager.cache.memoize(timeout=x) (we could have constants for slow, medium and long lived timouts) to ensure we don't leak memory on memoized methods that get called with very diverse parameters.
| def common_bootstrap_payload() -> Dict[str, Any]: | ||
| """Common data always sent to the client""" | ||
| @cache_manager.cache.memoize(timeout=60) | ||
| def common_bootstrap_payload(user: User) -> Dict[str, Any]: |
There was a problem hiding this comment.
I checked the docs, and while they appear to recommend not using mutable objects as the key, it should work in this case as __repr__() will always return the same value for a single user.
Memoization docs: https://flask-caching.readthedocs.io/en/latest/#memoization
…ache#21018) (apache#21439) Co-authored-by: Bogdan Kyryliuk <bogdankyryliuk@dropbox.com>
Try 2 on: #21018
Includes small refactoring of the
common_bootstrap_payloadthat passes user object to the function.SUMMARY
Memoize the common_bootstrap_payload. The function includes the config settings and the feature flags, those tend to be changed in frequently, this PR adds caching layer with 1 minute TTL. In our staging deployment it yields 10x speedup e.g. 22 ms down from ~250 ms
BEFORE
Screen Shot 2022-08-05 at 10 17 57 AM
AFTER
image
TESTING INSTRUCTIONS
[x] CI