Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
Reduce cache size by not storing deferreds #2158
Conversation
Some checks were not successful
Sytest Dendron (Commit)
— Build #1998 origin/erikj/reduce_cache_size succeeded in 9 min 2 sec
Sytest Postgres (Commit)
— Build #2828 origin/erikj/reduce_cache_size succeeded in 7 min 55 sec
erikjohnston
assigned
NegativeMjark
Apr 25, 2017
| @@ -335,20 +346,10 @@ def wrapped(*args, **kwargs): | ||
| try: | ||
| cached_result_d = cache.get(cache_key, callback=invalidate_callback) | ||
| - observer = cached_result_d.observe() | ||
| - if DEBUG_CACHES: |
NegativeMjark
Apr 25, 2017
Contributor
aw, I liked the read-through cache it was useful for debugging.
If you really want to remove it from here you should probably remove the DEBUG_CACHES variable entirely.
Some checks were not successful
Sytest Dendron (Commit)
— Build #2000 origin/erikj/reduce_cache_size succeeded in 8 min 41 sec
Sytest Postgres (Commit)
— Build #2830 origin/erikj/reduce_cache_size succeeded in 7 min 37 sec
Sytest SQLite (Commit)
— Build #2900 origin/erikj/reduce_cache_size succeeded in 6 min 25 sec
erikjohnston
merged commit b4da08c
into
develop
Apr 25, 2017
7 of 8 checks passed
Sytest Dendron (Merged PR)
Build finished.
Details
Sytest Dendron (Commit)
Build #2000 origin/erikj/reduce_cache_size succeeded in 8 min 41 sec
Details
Sytest Postgres (Commit)
Build #2830 origin/erikj/reduce_cache_size succeeded in 7 min 37 sec
Details
Sytest Postgres (Merged PR)
Build finished.
Details
Sytest SQLite (Commit)
Build #2900 origin/erikj/reduce_cache_size succeeded in 6 min 25 sec
Details
Sytest SQLite (Merged PR)
Build finished.
Details
continuous-integration/travis-ci/pr
The Travis CI build passed
Details
continuous-integration/travis-ci/push
The Travis CI build passed
Details
psaavedra
added a commit
to psaavedra/synapse
that referenced
this pull request
May 19, 2017
erikjohnston
deleted the
erikj/reduce_cache_size
branch
Oct 26, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
erikjohnston commentedApr 25, 2017
Currently the cache descriptors store deferreds rather than raw values, this is a simple way of triggering only one database hit and sharing the result if two callers attempt to get the same value.
However, there are a few caches that simply store a mapping from string to string (or int). These caches can have a large number of entries, under the assumption that each entry is small. However, the size of a
deferred (specifically the size of ObservableDeferred) is significantly larger than that of the raw value, 2kb vs 32b.
This PR therefore changes the cache descriptors to store the raw values rather than the deferreds.
As a side effect cached storage function now either return a deferred or the actual value, as the cached list decriptor already does. This is fine as we always end up just yield'ing on the returned value eventually, which handles that case correctly.