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

Speed up calculating push rules #2238

Merged
merged 3 commits into from May 22, 2017

Conversation

Projects
None yet
2 participants
Owner

erikjohnston commented May 19, 2017

No description provided.

synapse/push/bulk_push_rule_evaluator.py
if key[0] != EventTypes.Member:
continue
+ user_id = key[1]
@NegativeMjark

NegativeMjark May 22, 2017

Contributor

If we are optimising it might be worth noting that a, b = key seems to be faster than a = key[0], b = key[1]

synapse/push/bulk_push_rule_evaluator.py
@@ -200,6 +200,10 @@ def __init__(self, hs, room_id, rules_for_room_cache):
# not update the cache with it.
self.sequence = 0
+ # A cache of user_ids that we *know* aren't interesting, e.g. user_ids
+ # owned by AS's, or remote users, etc.
+ self.uninteresting_user_set = set()
@NegativeMjark

NegativeMjark May 22, 2017

Contributor

Could we have a comment explaining why this cache doesn't need invalidation.

@NegativeMjark

NegativeMjark May 22, 2017

Contributor

Also might be worth explaining that "uninteresting" means "will never have push rules on this server"

Contributor

NegativeMjark commented May 22, 2017

Other than commenting on the uninteresting_user_set LGTM.

erikjohnston added some commits May 22, 2017

@erikjohnston erikjohnston merged commit ccd6241 into develop May 22, 2017

6 checks passed

Sytest Dendron (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #3021 origin/erikj/faster_push_rules succeeded in 5 min 26 sec
Details
Sytest Postgres (Merged PR) Build finished.
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/faster_push_rules branch Oct 26, 2017

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