Performance for finding announcement for current user (and more)

[jhenkens]

I have been looking at rolling my own sitewide announcements for a site recently, and saw this gem linked from Stackoverflow and thought I'd at least check it out. I apologize for not breaking these up - I'm feeling lazy today.

In looking at your code for selecting announcements for a specific user, the process of reducing all possible announcements based on user attributes seems potentially very costly, in the rare case that there are a lot of tightly targeted active announcements. I can't think of an overall better way while maintaining that feature, but I did notice a few things that might be worthwhile to implement.

It might be worth considering checking the announcement attributes in batches, rather than loading all into memory at once.

I would also lazily create the user_as_array variable upon finding the first announcement that is targeted.

In your scope for ordering by start_delivery_at, I would perform a second sort based on creation date for those announcements where thestart_delivery_at is NULL, or where two have an equal date, but that would just be my preference

Lastly, and unrelated, have you thought about using cookies or the session to handle hiding announcements for non-current users? I'd been thinking of doing that and then merging the cookie into the database of viewed announcements upon signin.

[jhenkens]

I actually did a lot of what I talked about in d7dbb23a221387f067b7239571939a1caca0984c and at a first glance it seems to work. find_each ignores ordering so that cannot be done.

[csm123]

This is fantastic and I'll work to incorporate it into master. Performance is certainly critical and saving read announcements with a cookie if someone isn't logged in opens up new use cases. Thanks a bunch.

[jhenkens]

I've made more commits since then on my master branch. Feel free to check em out! Using it currently and I quite like it.

Sent from my mobile.

––––––––––––––––– Johan Henkens johan@henkens.com

On Nov 29, 2014, at 7:02 PM, Corey Martin notifications@github.com wrote:

This is fantastic and I'll work to incorporate it into master. Performance is certainly critical and saving read announcements with a cookie if someone isn't logged in opens up new use cases. Thanks a bunch.

— Reply to this email directly or view it on GitHub.

[csm123]

Fantastic. Your optimizations will help get this to 1.0

[csm123]

I incorporated indexing and uniqueness validation for 1.0. I plan to bring your feature for non-user-facing announcements into a later version - your implementation is really well-executed.

[csm123]

Keeping this open, as I'd like to get your performance enhancements into the next minor version.

[davidlesches]

I saw this on the Ruby Weekly and thought I'd just add my two cents to this enhancements thread.

This is a great feature and something I wanted to add often to my own sites, so having a gem is great. IMO though, the implementation is heavy compared to the benefit.

In the current implementation, the db is being hit on every single page load, and with multiple queries. IMO it's a costly hit for showing temporary announcement messages.

I think Redis is a better option than ActiveRecord for this. If anything, this seems to be one of those cases when Redis is an ideal option. It'll be a hundred times faster. The usual downside of Redis (that the data is often kept in memory and not committed to IO) isn't a concern here because the gem here deals with temporary announcements, not core app data. Should the server crash and some Redis data lost, nothing bad happens - just everyone sees a notice one more time. And the speed boost would be huge.

Anyway, I just think it would be a good feature to able to use Redis instead. If I find time I'll give it a pass and submit a pull request.

[csm123]

Redis support is a great idea. I would welcome a pull request to make this happen.

I definitely see your point about implementation cost. In practice, Starburst is in use on a site that gets over 40,000 sessions per month, and it doesn't even show up on New Relic's transaction list. The calculation of acceptable implementation burden will depend on the site, and how important announcements are to the site, but for my use case its cost has been minimal.

I benchmarked Starburst while it was development, albeit very simply. http://aspiringwebdev.com/creating-a-ruby-gem-for-one-time-announcements-part-4-performance-views/

I thought about caching, but this might not be the right use case, as the announcements a user sees can depend on the user's characteristics and whether they've read the message. Any ideas on that front are welcome, though.