From 79382f4948d4734711dda45e1ac397bf7149076c Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Mon, 10 Sep 2012 07:31:42 -0400 Subject: [PATCH] Speed up host lookup for cases where we don't need to do it, or where it ends up being too expensive when loading lists. --- clients/memcapable.cc | 5 +++- libmemcached/connect.cc | 52 +++++++++++++++++++++------------- libmemcached/hosts.cc | 55 ++++++++++++++++++++++++------------ libmemcached/instance.cc | 30 +++++++++++--------- libmemcached/memcached.cc | 2 +- libmemcached/server_list.cc | 35 +++++++++++------------ libmemcached/server_list.hpp | 2 +- libmemcachedutil/ping.cc | 35 ++++++++++++----------- libtest/include.am | 2 +- tests/include.am | 3 ++ 10 files changed, 131 insertions(+), 90 deletions(-) diff --git a/clients/memcapable.cc b/clients/memcapable.cc index bc3b551f..25b470b0 100644 --- a/clients/memcapable.cc +++ b/clients/memcapable.cc @@ -39,6 +39,8 @@ #include #include +#include + #ifdef linux /* /usr/include/netinet/in.h defines macros from ntohs() to _bswap_nn to * optimize the conversion functions, but the prototypes generate warnings @@ -1715,7 +1717,8 @@ static enum test_return test_ascii_mget(void) "test_ascii_mget4 test_ascii_mget5 " "test_ascii_mget6\r\n")); - char *returned[nkeys]; + std::vector returned; + returned.resize(nkeys); for (uint32_t x= 0; x < nkeys; ++x) { diff --git a/libmemcached/connect.cc b/libmemcached/connect.cc index 8d027df2..5d180d18 100644 --- a/libmemcached/connect.cc +++ b/libmemcached/connect.cc @@ -173,7 +173,8 @@ static memcached_return_t set_hostinfo(org::libmemcached::Instance* server) hints.ai_protocol= IPPROTO_TCP; } - server->address_info= NULL; + assert(server->address_info == NULL); + assert(server->address_info_next == NULL); int errcode; switch(errcode= getaddrinfo(server->hostname, str_port, &hints, &server->address_info)) { @@ -181,19 +182,49 @@ static memcached_return_t set_hostinfo(org::libmemcached::Instance* server) break; case EAI_AGAIN: + if (server->address_info) + { + freeaddrinfo(server->address_info); + server->address_info= NULL; + server->address_info_next= NULL; + } return memcached_set_error(*server, MEMCACHED_TIMEOUT, MEMCACHED_AT, memcached_string_make_from_cstr(gai_strerror(errcode))); case EAI_SYSTEM: + if (server->address_info) + { + freeaddrinfo(server->address_info); + server->address_info= NULL; + server->address_info_next= NULL; + } return memcached_set_errno(*server, errno, MEMCACHED_AT, memcached_literal_param("getaddrinfo(EAI_SYSTEM)")); case EAI_BADFLAGS: + if (server->address_info) + { + freeaddrinfo(server->address_info); + server->address_info= NULL; + server->address_info_next= NULL; + } return memcached_set_error(*server, MEMCACHED_INVALID_ARGUMENTS, MEMCACHED_AT, memcached_literal_param("getaddrinfo(EAI_BADFLAGS)")); case EAI_MEMORY: + if (server->address_info) + { + freeaddrinfo(server->address_info); + server->address_info= NULL; + server->address_info_next= NULL; + } return memcached_set_error(*server, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, memcached_literal_param("getaddrinfo(EAI_MEMORY)")); default: { + if (server->address_info) + { + freeaddrinfo(server->address_info); + server->address_info= NULL; + server->address_info_next= NULL; + } return memcached_set_error(*server, MEMCACHED_HOST_LOOKUP_FAILURE, MEMCACHED_AT, memcached_string_make_from_cstr(gai_strerror(errcode))); } } @@ -425,24 +456,7 @@ static memcached_return_t network_connect(org::libmemcached::Instance* server) { WATCHPOINT_ASSERT(server->state == MEMCACHED_SERVER_STATE_NEW); server->address_info_next= NULL; - memcached_return_t rc; - uint32_t counter= 5; - while (--counter) - { - if ((rc= set_hostinfo(server)) != MEMCACHED_TIMEOUT) - { - break; - } - -#ifndef WIN32 - struct timespec dream, rem; - - dream.tv_nsec= 1000; - dream.tv_sec= 0; - - nanosleep(&dream, &rem); -#endif - } + memcached_return_t rc= set_hostinfo(server); if (memcached_failed(rc)) { diff --git a/libmemcached/hosts.cc b/libmemcached/hosts.cc index 4875615f..61c1b059 100644 --- a/libmemcached/hosts.cc +++ b/libmemcached/hosts.cc @@ -352,17 +352,29 @@ static memcached_return_t server_add(memcached_st *ptr, { assert_msg(ptr, "Programmer mistake, somehow server_add() was passed a NULL memcached_st"); - org::libmemcached::Instance* new_host_list= libmemcached_xrealloc(ptr, memcached_instance_list(ptr), (ptr->number_of_hosts + 1), org::libmemcached::Instance); + if (ptr->number_of_hosts) + { + assert(memcached_instance_list(ptr)); + } + + if (memcached_instance_list(ptr)) + { + assert(ptr->number_of_hosts); + } + + uint32_t host_list_size= ptr->number_of_hosts +1; + org::libmemcached::Instance* new_host_list= libmemcached_xrealloc(ptr, memcached_instance_list(ptr), host_list_size, org::libmemcached::Instance); if (new_host_list == NULL) { return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT); } - memcached_instance_set(ptr, new_host_list); + memcached_instance_set(ptr, new_host_list, host_list_size); + assert(ptr->number_of_hosts == host_list_size); /* TODO: Check return type */ - org::libmemcached::Instance* instance= memcached_instance_fetch(ptr, memcached_server_count(ptr)); + org::libmemcached::Instance* instance= memcached_instance_fetch(ptr, memcached_server_count(ptr) -1); if (__instance_create_with(ptr, instance, hostname, port, weight, type) == NULL) { @@ -377,8 +389,6 @@ static memcached_return_t server_add(memcached_st *ptr, } } - ptr->number_of_hosts++; - return run_distribution(ptr); } @@ -390,23 +400,26 @@ memcached_return_t memcached_server_push(memcached_st *ptr, const memcached_serv return MEMCACHED_SUCCESS; } + uint32_t original_host_size= memcached_server_count(ptr); uint32_t count= memcached_server_list_count(list); + uint32_t host_list_size= count +original_host_size; - org::libmemcached::Instance* new_host_list= libmemcached_xrealloc(ptr, memcached_instance_list(ptr), (count + memcached_server_count(ptr)), org::libmemcached::Instance); + org::libmemcached::Instance* new_host_list= libmemcached_xrealloc(ptr, memcached_instance_list(ptr), host_list_size, org::libmemcached::Instance); if (new_host_list == NULL) { return MEMCACHED_MEMORY_ALLOCATION_FAILURE; } - memcached_instance_set(ptr, new_host_list); + memcached_instance_set(ptr, new_host_list, host_list_size); - for (uint32_t x= 0; x < count; x++) + ptr->state.is_parsing= true; + for (uint32_t x= 0; x < count; ++x, ++original_host_size) { WATCHPOINT_ASSERT(list[x].hostname[0] != 0); // We have extended the array, and now we will find it, and use it. - org::libmemcached::Instance* instance= memcached_instance_fetch(ptr, memcached_server_count(ptr)); + org::libmemcached::Instance* instance= memcached_instance_fetch(ptr, original_host_size); WATCHPOINT_ASSERT(instance); memcached_string_t hostname= { memcached_string_make_from_cstr(list[x].hostname) }; @@ -414,6 +427,7 @@ memcached_return_t memcached_server_push(memcached_st *ptr, const memcached_serv hostname, list[x].port, list[x].weight, list[x].type) == NULL) { + ptr->state.is_parsing= false; return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT); } @@ -421,9 +435,8 @@ memcached_return_t memcached_server_push(memcached_st *ptr, const memcached_serv { memcached_set_weighted_ketama(ptr, true); } - - ptr->number_of_hosts++; } + ptr->state.is_parsing= false; return run_distribution(ptr); } @@ -435,22 +448,28 @@ memcached_return_t memcached_instance_push(memcached_st *ptr, const struct org:: return MEMCACHED_SUCCESS; } - org::libmemcached::Instance* new_host_list= libmemcached_xrealloc(ptr, memcached_instance_list(ptr), (number_of_hosts +memcached_server_count(ptr)), org::libmemcached::Instance); + uint32_t original_host_size= memcached_server_count(ptr); + uint32_t host_list_size= number_of_hosts +original_host_size; + org::libmemcached::Instance* new_host_list= libmemcached_xrealloc(ptr, memcached_instance_list(ptr), host_list_size, org::libmemcached::Instance); if (new_host_list == NULL) { return MEMCACHED_MEMORY_ALLOCATION_FAILURE; } - memcached_instance_set(ptr, new_host_list); + memcached_instance_set(ptr, new_host_list, host_list_size); - for (uint32_t x= 0; x < number_of_hosts; x++) - { + // We don't bother with lookups for this operation + ptr->state.is_parsing= true; + // We use original_host_size since size will now point to the first new + // instance allocated. + for (uint32_t x= 0; x < number_of_hosts; ++x, ++original_host_size) + { WATCHPOINT_ASSERT(list[x].hostname[0] != 0); // We have extended the array, and now we will find it, and use it. - org::libmemcached::Instance* instance= memcached_instance_fetch(ptr, memcached_server_count(ptr)); + org::libmemcached::Instance* instance= memcached_instance_fetch(ptr, original_host_size); WATCHPOINT_ASSERT(instance); memcached_string_t hostname= { memcached_string_make_from_cstr(list[x].hostname) }; @@ -458,6 +477,7 @@ memcached_return_t memcached_instance_push(memcached_st *ptr, const struct org:: hostname, list[x].port(), list[x].weight, list[x].type) == NULL) { + ptr->state.is_parsing= false; return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT); } @@ -465,9 +485,8 @@ memcached_return_t memcached_instance_push(memcached_st *ptr, const struct org:: { memcached_set_weighted_ketama(ptr, true); } - - ptr->number_of_hosts++; } + ptr->state.is_parsing= false; return run_distribution(ptr); } diff --git a/libmemcached/instance.cc b/libmemcached/instance.cc index aacdad64..41f491e8 100644 --- a/libmemcached/instance.cc +++ b/libmemcached/instance.cc @@ -154,6 +154,7 @@ void __instance_free(org::libmemcached::Instance* self) self->address_info= NULL; self->address_info_next= NULL; } + assert(self->address_info_next == NULL); memcached_error_free(*self); @@ -307,23 +308,26 @@ static org::libmemcached::Instance* memcached_instance_clone(org::libmemcached:: void set_last_disconnected_host(org::libmemcached::Instance* self) { assert(self->root); - if (self->root == NULL) + if (self->root) { - return; - } + if (memcached_server_get_last_disconnect(self->root) and + memcached_server_get_last_disconnect(self->root)->version == self->version) + { + return; + } - if (memcached_server_get_last_disconnect(self->root) and - memcached_server_get_last_disconnect(self->root)->version == self->version) - { - return; - } + // const_cast + memcached_st *root= (memcached_st *)self->root; - // const_cast - memcached_st *root= (memcached_st *)self->root; + memcached_instance_free((org::libmemcached::Instance*)(root->last_disconnected_server)); - memcached_instance_free((org::libmemcached::Instance*)(root->last_disconnected_server)); - root->last_disconnected_server= memcached_instance_clone(self); - ((org::libmemcached::Instance*)memcached_server_get_last_disconnect(root))->version= self->version; + // We set is_parsing so that no lookup happens + root->state.is_parsing= true; + root->last_disconnected_server= memcached_instance_clone(self); + root->state.is_parsing= false; + + ((org::libmemcached::Instance*)memcached_server_get_last_disconnect(root))->version= self->version; + } } memcached_server_instance_st memcached_server_get_last_disconnect(const memcached_st *self) diff --git a/libmemcached/memcached.cc b/libmemcached/memcached.cc index 765b79ab..82d99f09 100644 --- a/libmemcached/memcached.cc +++ b/libmemcached/memcached.cc @@ -280,7 +280,7 @@ void memcached_servers_reset(memcached_st *self) { memcached_instance_list_free(memcached_instance_list(self), self->number_of_hosts); - memcached_instance_set(self, NULL); + memcached_instance_set(self, NULL, 0); self->number_of_hosts= 0; memcached_instance_free((org::libmemcached::Instance*)self->last_disconnected_server); self->last_disconnected_server= NULL; diff --git a/libmemcached/server_list.cc b/libmemcached/server_list.cc index 8205edc9..537e9edd 100644 --- a/libmemcached/server_list.cc +++ b/libmemcached/server_list.cc @@ -127,39 +127,36 @@ uint32_t memcached_instance_list_count(const memcached_st* self) : self->number_of_hosts; } -void memcached_instance_set(memcached_st* memc, org::libmemcached::Instance* list) +void memcached_instance_set(memcached_st* memc, org::libmemcached::Instance* list, const uint32_t host_list_size) { memc->servers= list; + memc->number_of_hosts= host_list_size; } void memcached_server_list_free(memcached_server_list_st self) { - if (self == NULL) + if (self) { - return; - } + for (uint32_t x= 0; x < memcached_server_list_count(self); x++) + { + assert_msg(not memcached_is_allocated(&self[x]), "You have called memcached_server_list_free(), but you did not pass it a valid memcached_server_list_st"); + __server_free(&self[x]); + } - for (uint32_t x= 0; x < memcached_server_list_count(self); x++) - { - assert_msg(not memcached_is_allocated(&self[x]), "You have called memcached_server_list_free(), but you did not pass it a valid memcached_server_list_st"); - __server_free(&self[x]); + libmemcached_free(self->root, self); } - - libmemcached_free(self->root, self); } void memcached_instance_list_free(org::libmemcached::Instance* self, uint32_t instance_count) { - if (self == NULL) + if (self) { - return; - } + for (uint32_t x= 0; x < instance_count; x++) + { + assert_msg(memcached_is_allocated(&self[x]) == false, "You have called memcached_server_list_free(), but you did not pass it a valid memcached_server_list_st"); + __instance_free(&self[x]); + } - for (uint32_t x= 0; x < instance_count; x++) - { - assert_msg(not memcached_is_allocated(&self[x]), "You have called memcached_server_list_free(), but you did not pass it a valid memcached_server_list_st"); - __instance_free(&self[x]); + libmemcached_free(self->root, self); } - - libmemcached_free(self->root, self); } diff --git a/libmemcached/server_list.hpp b/libmemcached/server_list.hpp index 95920194..ffbb6a36 100644 --- a/libmemcached/server_list.hpp +++ b/libmemcached/server_list.hpp @@ -44,4 +44,4 @@ uint32_t memcached_servers_set_count(memcached_server_list_st servers, uint32_t void memcached_instance_list_free(org::libmemcached::Instance* self, uint32_t count); -void memcached_instance_set(memcached_st*, org::libmemcached::Instance*); +void memcached_instance_set(memcached_st*, org::libmemcached::Instance*, const uint32_t host_list_size); diff --git a/libmemcachedutil/ping.cc b/libmemcachedutil/ping.cc index 0154b530..e2831beb 100644 --- a/libmemcachedutil/ping.cc +++ b/libmemcachedutil/ping.cc @@ -54,30 +54,31 @@ bool libmemcached_util_ping(const char *hostname, in_port_t port, memcached_retu return false; } - (void)memcached_behavior_set(memc_ptr, MEMCACHED_BEHAVIOR_CONNECT_TIMEOUT, 400000); - - memcached_return_t rc= memcached_server_add(memc_ptr, hostname, port); - if (memcached_success(rc)) - { - rc= memcached_version(memc_ptr); - } - - if (memcached_failed(rc) and rc == MEMCACHED_SOME_ERRORS) + if (memcached_success((*ret= memcached_behavior_set(memc_ptr, MEMCACHED_BEHAVIOR_CONNECT_TIMEOUT, 400000)))) { - memcached_server_instance_st instance= - memcached_server_instance_by_position(memc_ptr, 0); + memcached_return_t rc= memcached_server_add(memc_ptr, hostname, port); + if (memcached_success(rc)) + { + rc= memcached_version(memc_ptr); + } - assert_msg(instance and memcached_server_error(instance), " "); - if (instance and memcached_server_error(instance)) + if (memcached_failed(rc) and rc == MEMCACHED_SOME_ERRORS) { - rc= memcached_server_error_return(instance); + memcached_server_instance_st instance= + memcached_server_instance_by_position(memc_ptr, 0); + + assert_msg(instance and memcached_server_error(instance), " "); + if (instance and memcached_server_error(instance)) + { + rc= memcached_server_error_return(instance); + } } + + *ret= rc; } memcached_free(memc_ptr); - *ret= rc; - - return memcached_success(rc); + return memcached_success(*ret); } bool libmemcached_util_ping2(const char *hostname, in_port_t port, const char *username, const char *password, memcached_return_t *ret) diff --git a/libtest/include.am b/libtest/include.am index 28c89c59..3dd6982a 100644 --- a/libtest/include.am +++ b/libtest/include.am @@ -5,7 +5,7 @@ # LIBTOOL_COMMAND= ${abs_top_builddir}/libtool --mode=execute -VALGRIND_EXEC_COMMAND= $(LIBTOOL_COMMAND) valgrind --error-exitcode=1 --leak-check=yes --show-reachable=yes --track-fds=yes --malloc-fill=A5 --free-fill=DE --xml-file=./tmp_chroot/var/tmp/yatl-\%p.xml --xml=yes +VALGRIND_EXEC_COMMAND= $(LIBTOOL_COMMAND) valgrind --error-exitcode=1 --leak-check=yes --show-reachable=yes --track-fds=yes --malloc-fill=A5 --free-fill=DE VALGRIND_COMMAND= TESTS_ENVIRONMENT="valgrind" $(VALGRIND_EXEC_COMMAND) HELGRIND_COMMAND= $(LIBTOOL_COMMAND) valgrind --tool=helgrind --read-var-info=yes --error-exitcode=1 --read-var-info=yes DRD_COMMAND= $(LIBTOOL_COMMAND) valgrind --tool=drd diff --git a/tests/include.am b/tests/include.am index b8e9e17f..2538d51c 100644 --- a/tests/include.am +++ b/tests/include.am @@ -107,6 +107,9 @@ test-cycle: tests/cycle test-memcapable: tests/memcapable @tests/memcapable +valgrind-memcapable: tests/memcapable + $(VALGRIND_COMMAND) tests/memcapable + pahole-mem: tests/testapp @$(PAHOLE_COMMAND) tests/testapp -- 2.30.2