Skip to content
This repository was archived by the owner on Apr 8, 2026. It is now read-only.

add members_only option. #10

Merged
czarneckid merged 7 commits into
WB-Games:masterfrom
simonklee:members_only-option
May 15, 2013
Merged

add members_only option. #10
czarneckid merged 7 commits into
WB-Games:masterfrom
simonklee:members_only-option

Conversation

@simonklee

Copy link
Copy Markdown

Haven't added tests for this pull request yet, waiting for an OK from you.

The idea here is that in some cases you are really just interested in the member of the sorted set and you don't care about it's score or rank. In those cases the ranking function will have a negative impact on performance, while the results of it are discarded.

This case applies to your Ruby activity_feed library. In my Python port I could see the following difference by adding this option.

before:
before

after:
after

@czarneckid

Copy link
Copy Markdown
Contributor

That seems like a reasonable feature. 👍

@simonklee

Copy link
Copy Markdown
Author

please take another look

@czarneckid

Copy link
Copy Markdown
Contributor

(replace Hash w/ Dictionary for Python here, just thinking about then porting this to the Ruby and CoffeeScript libraries)

One thought here, the current behavior is that you're always getting an array of Hash objects, but with the members_only parameter you'd be returning an array of String objects. Would we still want to keep the array of Hash where the only Hash key is member or this is a special case with an array of String objects? I guess to your point above, with respect to performance, it might be overkill to take the array of String objects and create an array of Hash objects.

@simonklee

Copy link
Copy Markdown
Author

From a consistency standpoint returning a hash/dict is probably preferable. Furthermore, the speedup was not primary because I dropped the dict allocation, but because I dropped 1 round-trip to the redis server which included N * 2 commands, where N was the size of the result set.

The difference between allocating a dict for each member and not doing so is something in the range of 10 µs.

I'll update my tests and make new commit with these changes.

czarneckid added a commit that referenced this pull request May 15, 2013
@czarneckid czarneckid merged commit abe670e into WB-Games:master May 15, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants