Skip to content

Commit

Permalink
conditionally don't inline the ASCII VALUE line
Browse files Browse the repository at this point in the history
missing methods of changing the value. uses what should be a high speed itoa
instead of snprintf in a handful of places. Should speed up sets, reduce
memory overhead, and ideally not slow down gets too much.
  • Loading branch information
dormando committed Jan 8, 2017
1 parent 1cc77e6 commit c2027ad
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 16 deletions.
14 changes: 11 additions & 3 deletions items.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,12 @@ static unsigned int noexp_lru_size(int slabs_clsid) {
*/
static size_t item_make_header(const uint8_t nkey, const unsigned int flags, const int nbytes,
char *suffix, uint8_t *nsuffix) {
/* suffix is defined at 40 chars elsewhere.. */
*nsuffix = (uint8_t) snprintf(suffix, 40, " %u %d\r\n", flags, nbytes - 2);
if (settings.inline_ascii_response) {
/* suffix is defined at 40 chars elsewhere.. */
*nsuffix = (uint8_t) snprintf(suffix, 40, " %u %d\r\n", flags, nbytes - 2);
} else {
*nsuffix = sizeof(flags);
}
return sizeof(item) + nkey + *nsuffix + nbytes;
}

Expand Down Expand Up @@ -242,7 +246,11 @@ item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags,
it->nbytes = nbytes;
memcpy(ITEM_key(it), key, nkey);
it->exptime = exptime;
memcpy(ITEM_suffix(it), suffix, (size_t)nsuffix);
if (settings.inline_ascii_response) {
memcpy(ITEM_suffix(it), suffix, (size_t)nsuffix);
} else {
memcpy(ITEM_suffix(it), &flags, sizeof(flags));
}
it->nsuffix = nsuffix;

/* Need to shuffle the pointer stored in h_next into it->data. */
Expand Down
2 changes: 1 addition & 1 deletion itoa_ljust.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ static inline char* itoa(uint32_t u, char* p, int d, int n) {
}

char* itoa_u32(uint32_t u, char* p) {
int d,n;
int d = 0,n;
if (u >=100000000) n = digits(u, 100000000, &d, &p, 10);
else if (u < 100) n = digits(u, 1, &d, &p, 2);
else if (u < 10000) n = digits(u, 100, &d, &p, 4);
Expand Down
47 changes: 39 additions & 8 deletions memcached.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ static void settings_init(void) {
settings.hot_lru_pct = 32;
settings.warm_lru_pct = 32;
settings.expirezero_does_not_evict = false;
settings.inline_ascii_response = false;
settings.idle_timeout = 0; /* disabled */
settings.hashpower_init = 0;
settings.slab_reassign = false;
Expand Down Expand Up @@ -1507,7 +1508,11 @@ static void process_bin_get_or_touch(conn *c) {
rsp->message.header.response.cas = htonll(ITEM_get_cas(it));

// add the flags
rsp->message.body.flags = htonl(strtoul(ITEM_suffix(it), NULL, 10));
if (settings.inline_ascii_response) {
rsp->message.body.flags = htonl(strtoul(ITEM_suffix(it), NULL, 10));
} else {
rsp->message.body.flags = htonl(*((uint32_t *)ITEM_suffix(it)));
}
add_iov(c, &rsp->message.body, sizeof(rsp->message.body));

if (should_return_key) {
Expand Down Expand Up @@ -2656,7 +2661,11 @@ enum store_item_type do_store_item(item *it, int comm, conn *c, const uint32_t h
/* we have it and old_it here - alloc memory to hold both */
/* flags was already lost - so recover them from ITEM_suffix(it) */

flags = (uint32_t) strtoul(ITEM_suffix(old_it), (char **) NULL, 10);
if (settings.inline_ascii_response) {
flags = (uint32_t) strtoul(ITEM_suffix(old_it), (char **) NULL, 10);
} else {
flags = *((uint32_t *)ITEM_suffix(old_it));
}

new_it = do_item_alloc(key, it->nkey, flags, old_it->exptime, it->nbytes + old_it->nbytes - 2 /* CRLF */);

Expand Down Expand Up @@ -3179,6 +3188,22 @@ static void process_stat(conn *c, token_t *tokens, const size_t ntokens) {
}
}

static inline int make_ascii_get_suffix(char *suffix, item *it, bool return_cas) {
char *p;
*suffix = ' ';
p = itoa_u32(*((uint32_t *) ITEM_suffix(it)), suffix+1);
*p = ' ';
p = itoa_u32(it->nbytes-2, p+1);
if (return_cas) {
*p = ' ';
p = itoa_u64(ITEM_get_cas(it), p+1);
}
*p = '\r';
*(p+1) = '\n';
*(p+2) = '\0';
return (p - suffix) + 2;
}

/* ntokens is overwritten here... shrug.. */
static inline void process_get_command(conn *c, token_t *tokens, size_t ntokens, bool return_cas) {
char *key;
Expand Down Expand Up @@ -3230,7 +3255,7 @@ static inline void process_get_command(conn *c, token_t *tokens, size_t ntokens,
* " " + flags + " " + data length + "\r\n" + data (with \r\n)
*/

if (return_cas)
if (return_cas || !settings.inline_ascii_response)
{
MEMCACHED_COMMAND_GET(c->sfd, ITEM_key(it), it->nkey,
it->nbytes, ITEM_get_cas(it));
Expand Down Expand Up @@ -3263,12 +3288,13 @@ static inline void process_get_command(conn *c, token_t *tokens, size_t ntokens,
return;
}
*(c->suffixlist + i) = suffix;
int suffix_len = snprintf(suffix, SUFFIX_SIZE,
/*int suffix_len = snprintf(suffix, SUFFIX_SIZE,
" %llu\r\n",
(unsigned long long)ITEM_get_cas(it));
(unsigned long long)ITEM_get_cas(it));*/
int suffix_len = make_ascii_get_suffix(suffix, it, return_cas);
//add_iov(c, ITEM_suffix(it), it->nsuffix - 2) != 0 ||
if (add_iov(c, "VALUE ", 6) != 0 ||
add_iov(c, ITEM_key(it), it->nkey) != 0 ||
add_iov(c, ITEM_suffix(it), it->nsuffix - 2) != 0 ||
add_iov(c, suffix, suffix_len) != 0)
{
item_remove(it);
Expand Down Expand Up @@ -3348,7 +3374,7 @@ static inline void process_get_command(conn *c, token_t *tokens, size_t ntokens,

c->icurr = c->ilist;
c->ileft = i;
if (return_cas) {
if (return_cas || !settings.inline_ascii_response) {
c->suffixcurr = c->suffixlist;
c->suffixleft = i;
}
Expand Down Expand Up @@ -3639,7 +3665,12 @@ enum delta_result_type do_add_delta(conn *c, const char *key, const size_t nkey,
do_item_update(it);
} else if (it->refcount > 1) {
item *new_it;
uint32_t flags = (uint32_t) strtoul(ITEM_suffix(it)+1, (char **) NULL, 10);
uint32_t flags;
if (settings.inline_ascii_response) {
flags = (uint32_t) strtoul(ITEM_suffix(it)+1, (char **) NULL, 10);
} else {
flags = *((uint32_t *)ITEM_suffix(it));
}
new_it = do_item_alloc(ITEM_key(it), it->nkey, flags, it->exptime, res + 2);
if (new_it == 0) {
do_item_remove(it);
Expand Down
6 changes: 3 additions & 3 deletions memcached.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@
#define UDP_MAX_PAYLOAD_SIZE 1400
#define UDP_HEADER_SIZE 8
#define MAX_SENDBUF_SIZE (256 * 1024 * 1024)
/* I'm told the max length of a 64-bit num converted to string is 20 bytes.
* Plus a few for spaces, \r\n, \0 */
#define SUFFIX_SIZE 24
/* Up to 3 numbers (2 32bit, 1 64bit), spaces, newlines, null 0 */
#define SUFFIX_SIZE 50

/** Initial size of list of items being returned by "get". */
#define ITEM_LIST_INITIAL 200
Expand Down Expand Up @@ -369,6 +368,7 @@ struct settings {
int warm_lru_pct; /* percentage of slab space for WARM_LRU */
int crawls_persleep; /* Number of LRU crawls to run before sleeping */
bool expirezero_does_not_evict; /* exptime == 0 goes into NOEXP_LRU */
bool inline_ascii_response; /* pre-format the VALUE line for ASCII responses */
int idle_timeout; /* Number of seconds to let connections idle */
unsigned int logger_watcher_buf_size; /* size of logger's per-watcher buffer */
unsigned int logger_buf_size; /* size of per-thread logger buffer */
Expand Down
3 changes: 2 additions & 1 deletion t/issue_42.t
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ for ($key = 0; $key < 10; $key++) {

my $first_stats = mem_stats($sock, "slabs");
my $req = $first_stats->{"1:mem_requested"};
ok ($req == "640" || $req == "800", "Check allocated size");
print STDERR "REQ: $req\n";
ok ($req == "640" || $req == "800" || $req == "770", "Check allocated size");

0 comments on commit c2027ad

Please sign in to comment.