Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Several small perf improvements for realms with large numbers of users #392

Closed
wants to merge 7 commits into from

Conversation

timabbott
Copy link
Member

Thanks @dbiollo for these suggestions (see #375)!

This fixes a performance issue looking up UserProfile objects for
realms with a large number of users in the case that a UserProfile
object is not in the cache.

Thanks to @dbiollo for the suggestion!
@smarx
Copy link

smarx commented Dec 26, 2015

Automated message from Dropbox CLA bot

@timabbott, it looks like you've already signed the Dropbox CLA. Thanks!

Since we frequently do filters for only active users, these indexes
may help performance in some cases.
This fixes a performance issue joining a server with a large number of
users.

Thanks to @dbiollo for the suggestion!
This makes it possible to use DevAuthBackend when doing
performance/scalability testing on Zulip with many thousands of users.

It's unlikely that anyone testing this backend will find it valuable
to have more than 100 login buttons on the same page, and if they do,
they can always just change this limit.

Thanks to @dbiollo for the suggestion!
Just doing the database query is more readable, and has about the same
performance as before in the case where active user dicts for the
realm are in cache (and is substantially better in the rare case that
this isn't in the cache).

Thanks to @dbiollo for the perf investigation and suggestion!
This removes from our cache a moderate amount of totally useless alert
word data corresponding to users who don't have any alert words.

Thanks to @dbiollo for the suggestion!
The browser registers for events via loading the home view, not this
interface, and this functionality is available via the API-format
register route anyway.
@timabbott
Copy link
Member Author

(The test failure is a flake and should be fixed by #394 )

@dbiollo
Copy link

dbiollo commented Jan 4, 2016

@timabbott - I tested the tweaks with my large dataset and it works well as expected! Thanks for implementing the optimizations.

@lfranchi
Copy link
Member

lgtm fftp

@timabbott
Copy link
Member Author

Merged, thanks for the review!

@timabbott timabbott closed this Jan 10, 2016
@timabbott timabbott deleted the perf branch January 10, 2016 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants