Faster get multi rebased #341

Merged
merged 4 commits into from May 26, 2013

Projects

None yet

2 participants

@joerixaop
Collaborator

Another performance related improvement.
Not quite as spectacular, but the bench rake task does seem to confirm my guess that it's faster.

I changed the require's to require_relative (when that method is defined), because I spent quite a bit of time trying to figure out why my changes had no effect in irb, before discovering that it required the gem versions of the files. If necessary I guess that can be removed from this branch

Collaborator
mperham commented Mar 13, 2013

Ugh, yeah rollback those require changes. We can discuss in a separate PR if you want them integrated.

Collaborator

Whoops, that would have worked if it was defined?(require_relative) instead of defined?(:symbol_that_should_have_been_method_call)

joerixaop added some commits Mar 7, 2013
@joerixaop joerixaop Pipeline requests in get_multi
Instead of doing lot's of small writes do 1 larger write. This also
considerably cuts down on locking already locked servers.
a5697a2
@joerixaop joerixaop Keep existing behaviour when no server is found 792ec01
@joerixaop joerixaop Debug message instead of warning
Closer to old behaviour
0766b39
Collaborator
mperham commented Mar 13, 2013

Do you have performance metrics showing an improvement?

Collaborator

rake bench (on 1.9.3-p392), with kgio seems to be running about 0.4s faster
for each of the get_multi/read_multi tests. Without kgio it seems to be 0.3s

That's with only a few keys. In the dataset that I used last time (well,
not exactly the same dataset) it seems the performance improvement is more
noticeable, but I didn't test that with the latest versions of my patch
(takes longer to develop)

That being said I forgot about making sure that the hook method #perform
can still be instrumented. So don't merge it yet, I need to update that
part.

On Wed, Mar 13, 2013 at 9:33 PM, Mike Perham notifications@github.comwrote:

Do you have performance metrics showing an improvement?


Reply to this email directly or view it on GitHubhttps://github.com/mperham/dalli/pull/341#issuecomment-14866700
.

@mperham mperham merged commit 1693577 into petergoldstein:master May 26, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment