larvalabs / breaker

Chat rooms for subreddits.
http://www.breakerapp.com
20 stars 3 forks source link

Store active rooms in a scored redis set #43

Closed megamattron closed 8 years ago

megamattron commented 8 years ago

Continuously update set as messages are sent. This avoids expensive queries when building initial state. NOTE! This might have some bugs, I've only done initial testing.

@KenavR This is my proposed optimization for the active rooms situation. It has a couple major upsides:

  1. Faster execution
  2. I'm doing the removal of active rooms that you're in during the loop over your chat room joins that we execute during initial state building anyway, so it saves additional iteration of the joins which can be expensive.
  3. Simpler code

It also has a couple downsides:

  1. I'm not calculating a number of new messages for the active rooms, but I'm not sure that's critical. People are finding the red message count number distracting anyway.
  2. I think the ranking might be off, I'm not sure I'm sorting this that correctly at the moment.

i'm sorry there's a lot of deleted code here, I just removed anything that didn't seem critical to this approach so I wasn't confused.

In terms of code structure, in addition to the changes needed for the stuff above, I've removed a few of those service objects and other extra classes. They don't seem to provide much value and make things a little harder to figure out when you're following the code. So in the future it'd be easier for me if we stuck to Controllers, Models, Json, Jobs, Util classes and stuff like that. Don't worry about it too much, but in general it's not usually that necessary to add a class.

megamattron commented 8 years ago

I fixed up some bugs and everything and am now merging this into master for release. I think this should solve the performance problems we're seeing.