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

Make presence.get_new_events a bit faster #1876

Merged
merged 5 commits into from Feb 2, 2017

Conversation

Projects
None yet
2 participants
Owner

erikjohnston commented Feb 2, 2017

We do this by caching the set of users a user shares rooms with.

erikjohnston added some commits Feb 2, 2017

Make presence.get_new_events a bit faster
We do this by caching the set of users a user shares rooms with.
@@ -280,6 +280,23 @@ def get_rooms_for_user(self, user_id):
user_id, membership_list=[Membership.JOIN],
)
+ @cachedInlineCallbacks(max_entries=50000, cache_context=True, iterable=True)
+ def get_users_who_share_room_with_user(self, user_id, cache_context):
@NegativeMjark

NegativeMjark Feb 2, 2017

Contributor

I guess we are leaking the cache_context.invalidate callbacks if this cache is smaller than the get_users_in_room or get_rooms_for_user caches. I wonder if we need a max_entries at all for dependant caches like this one since it can't be bigger than the get_rooms_for_user or get_users_in_room caches.

synapse/util/caches/descriptors.py
@@ -478,6 +478,8 @@ def update_results_dict(res):
class _CacheContext(namedtuple("_CacheContext", ("cache", "key"))):
+ # We rely on _CacheContext implementing __eq__ and __hash__ sensibly,
+ # which namedtuple does for us.
@NegativeMjark

NegativeMjark Feb 2, 2017

Contributor

Maybe expound on the nature of the reliance?

@NegativeMjark

NegativeMjark Feb 2, 2017 edited

Contributor

(I'm worried that future me will find this comment and will have forgotten what bit of code relies on these having __eq__ and __hash__ defined "sensibly" or what sensible means in this case)

Contributor

NegativeMjark commented Feb 2, 2017

Other than the sightly cryptic comment LGTM.

@erikjohnston erikjohnston merged commit a833189 into develop Feb 2, 2017

7 of 8 checks passed

Sytest Dendron (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #1480 origin/erikj/shared_member_store succeeded in 10 min
Details
Sytest Postgres (Commit) Build #2300 origin/erikj/shared_member_store succeeded in 7 min 14 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2368 origin/erikj/shared_member_store succeeded in 5 min 14 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

@erikjohnston erikjohnston deleted the erikj/shared_member_store branch Mar 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment