More pipelining

[erichummel]

hi there :). first off thanks so much for building this and sharing it with the world. i'm building a leaderboard and this gem made the process so much easier.

when I first set this up, I noticed that it's rather chatty fetching things from redis - some calls are pipelined and some are not, most notably the hgets and zcounts. so... I tweaked it to use hmget where appropriate and to pipeline the zcounts where possible, and the result is that the endpoint we have serving up our leaderboards has response times that are ~20% of what they were before these tweaks (we serve up 3 different leaderboards and display both the top 100 and the ten players around a given logged in player).

i tried to follow the style/documentation y'all used, feel free to let me know if it's not up to snuff, I'm happy to fix it if it isn't. the specs all pass.

cheers and thanks again!

[czarneckid]

@erichummel Looks like a worthwhile set of changes. A couple of things:

1. Would you mind adding a members_data_for(members) method that just passes @leaderboard_name to the members_data_for_in(leaderboard_name, members) method? That's been the pattern with other similar methods.
2. Can you squash the changes into a single commit?

Thanks again for your contribution.

[erichummel]

sure thing! i'll get that done shortly

[erichummel]

okay, i created members_data_for, and also renamed the method i created from rankings_for_members_in to rankings_for_members_having_scores_in for a little more clarity as to what that method does and what it needs as input (and created the delegating method without the _in as well).

I'm not really sure if the rankings_for_members_having_scores_in method should be a public method, since it's really just a helper method that makes pipelining to get the ranks of members possible, and its inputs expose a bit of an implementation detail. On the other hand, I don't think it's that dangerous. someone else using it, however, would need to be warned that the scores passed in must align with the scores of the members passed in as they exist in redis. If that's not the case the return value of that method is essentially pure fiction.

Thoughts?

[czarneckid]

I think I'd rather see the rankings_for_members_having_scores_in method be protected ala the member_data_key(...) or validate_page_size(...) methods. And since it'll be a helper method, no need for the ..._in(...) or delegating method.

One last thing, there's a blank line between the method documentation and the definition for the rankings_for_members_having_scores and rankings_for_members_having_scores_in methods. You can remove those.

[erichummel]

okey dokey. i removed the delegating method, made the helper method protected and fixed the whitespace. I kept the _in in the method name, since that pattern's pretty standard in this code when a leaderboard_name is an injected dependency, rather than relying on the @leaderboard_name instance variable.

if I have time later this week i'll look into porting this over to the coffeescript and python ports of this gem if you'd like.

[erichummel]

just checking in on this - we've been running this live for a few weeks and it's performing swimmingly.

[czarneckid]

Great to hear. I think I saw in your branch there was one more commit that wasn't pushed to this PR? Is that something you also want reviewed or no?

Sorry for the delay, but I missed our last Hack-a-Thon to get this in. At the very least I need to devote some time to get similar changes into the python leaderboard library when I integrate this change.

[erichummel]

I did tweak something (it was minor, and I've already forgotten what it was exactly), but I squashed it into this commit so it should be good to go