no reply patch

dormando dormando at rydia.net
Fri Feb 22 05:07:31 UTC 2008


Committed this to trunk.

I didn't add the unlikely() stuff yet, because we haven't explicitly
dropped support for gcc-2.95, and I'm not familiar enough with its
portability to say if it'd work for sun or not.

After this release though, we should probably drop support for gcc-2.95,
which means freebsd4 users (*cough*) really ought to build memcached
with a newer gcc. Although it'd be great if we could ditch it *for*
release 1.2.5, so speak up if you care.

-Dormando

Tomash Brechko wrote:
> On Tue, Feb 19, 2008 at 00:44:26 -0800, dormando wrote:
>> For sanity I'm going to have to change it to look like this:
>>
>>     if (c->noreply) {
>>         if (settings.verbose > 1)
>>             fprintf(stderr, ">%d NOREPLY %s\n", c->sfd, str);
>>         c->noreply = false;
>>         conn_set_state(c, conn_read);
>>         return;
>>     }
> 
> Ok, no objections.
> 
> 
>> ...and for both of these, the test should be 'if unlikely(c->noreply)' -
> 
> It hardly would make any difference, we are not in a tight loop.  You
> could also write unlikely(settings.verbose > 1), but I'd say one
> should save such optimizations to places there they do make noticeable
> speedup.  OTOH, I guess GCC's __builtin_expect() may suggest the
> compiler to reverse the order of branches.
> 
> 
>> This also needs some more careful documentation warning about the
>> occasional bad line potentially throwing clients out of whack. Since
>> this is my pet peeve I'll add it myself.
> 
> As was mentioned before, the problem is not limited to 'noreply' only.
> Send to memcached "SET KEY\r\nVALUE\r\n" (in upper case), and enjoy
> two replies instead of one.  With 'norepy' it's just one reply instead
> of zero, same problem.  And this can only happen for malformed
> command, where error reply is send before the command has been parsed
> (memcached couldn't parse it).
> 
> 



More information about the memcached mailing list