Updated Branches: refs/heads/master 6636f4dcf -> 178c7b92a
TS-2570: Reduce number of malloc/free in remap_stats fast path Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/178c7b92 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/178c7b92 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/178c7b92 Branch: refs/heads/master Commit: 178c7b92a42d3eead77143f78bcf60afbb6d3951 Parents: 6636f4d Author: Phil Sorber <[email protected]> Authored: Thu Feb 13 16:09:13 2014 -0700 Committer: Phil Sorber <[email protected]> Committed: Thu Feb 13 16:09:36 2014 -0700 ---------------------------------------------------------------------- plugins/experimental/remap_stats/remap_stats.c | 108 ++++++++------------ 1 file changed, 42 insertions(+), 66 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/178c7b92/plugins/experimental/remap_stats/remap_stats.c ---------------------------------------------------------------------- diff --git a/plugins/experimental/remap_stats/remap_stats.c b/plugins/experimental/remap_stats/remap_stats.c index 246796f..04b1723 100644 --- a/plugins/experimental/remap_stats/remap_stats.c +++ b/plugins/experimental/remap_stats/remap_stats.c @@ -23,6 +23,7 @@ #include "ink_defs.h" #include "ts/ts.h" +#include <stdint.h> #include <stdbool.h> #include <string.h> #include <stdio.h> @@ -32,6 +33,8 @@ #define PLUGIN_NAME "remap_stats" #define DEBUG_TAG PLUGIN_NAME +#define MAX_STAT_LENGTH (1<<8) + typedef struct { bool post_remap_host; @@ -40,16 +43,10 @@ typedef struct TSMutex stat_creation_mutex; } config_t; -typedef struct -{ - char *hostname; - bool remap_success; -} txn_data_t; - static void stat_add(char *name, TSMgmtInt amount, TSStatPersistence persist_type, TSMutex create_mutex) { - int stat_id = -1, *statp; + intptr_t stat_id = -1; ENTRY search, *result = NULL; static __thread struct hsearch_data stat_cache; static __thread bool hash_init = false; @@ -70,30 +67,28 @@ stat_add(char *name, TSMgmtInt amount, TSStatPersistence persist_type, TSMutex c // so this mutex won't be much overhead and it fixes a race condition // in the RecCore. Hopefully this can be removed in the future. TSMutexLock(create_mutex); - if (TS_ERROR == TSStatFindName((const char *) name, &stat_id)) + if (TS_ERROR == TSStatFindName((const char *) name, (int *)&stat_id)) { stat_id = TSStatCreate((const char *) name, TS_RECORDDATATYPE_INT, persist_type, TS_STAT_SYNC_SUM); if (stat_id == TS_ERROR) TSDebug(DEBUG_TAG, "Error creating stat_name: %s", name); else - TSDebug(DEBUG_TAG, "Created stat_name: %s stat_id: %d", name, stat_id); + TSDebug(DEBUG_TAG, "Created stat_name: %s stat_id: %d", name, (int)stat_id); } TSMutexUnlock(create_mutex); search.key = TSstrdup(name); - statp = TSmalloc(sizeof(int)); - *statp = stat_id; - search.data = (void *) statp; + search.data = (void *) stat_id; hsearch_r(search, ENTER, &result, &stat_cache); - TSDebug(DEBUG_TAG, "Cached stat_name: %s stat_id: %d", name, stat_id); + TSDebug(DEBUG_TAG, "Cached stat_name: %s stat_id: %d", name, (int)stat_id); } else - stat_id = *((int *) result->data); + stat_id = (intptr_t) result->data; if (likely(stat_id >= 0)) - TSStatIntIncrement(stat_id, amount); + TSStatIntIncrement((int)stat_id, amount); else - TSDebug(DEBUG_TAG, "stat error! stat_name: %s stat_id: %d", name, stat_id); + TSDebug(DEBUG_TAG, "stat error! stat_name: %s stat_id: %d", name, (int)stat_id); } static char * @@ -118,30 +113,16 @@ get_effective_host(TSHttpTxn txn) return tmp; } -static char * -create_stat_name(char *hostname, char *basename) -{ - char *stat_name; - size_t stat_len; - - stat_len = strlen(hostname) + strlen(basename) + strlen(PLUGIN_NAME) + 3 + strlen("plugin."); - stat_name = TSmalloc(stat_len * sizeof(char)); - snprintf(stat_name, stat_len, "plugin.%s.%s.%s", PLUGIN_NAME, hostname, basename); - return stat_name; -} - static int handle_read_req_hdr(TSCont cont, TSEvent event ATS_UNUSED, void *edata) { TSHttpTxn txn = (TSHttpTxn) edata; config_t *config; - txn_data_t *txnd; + void *txnd; config = (config_t *) TSContDataGet(cont); - txnd = TSmalloc(sizeof(txn_data_t)); - txnd->remap_success = false; - txnd->hostname = get_effective_host(txn); - TSHttpTxnArgSet(txn, config->txn_slot, (void *) txnd); + txnd = (void *) get_effective_host(txn); // low bit left 0 because we do not know that remap succeeded yet + TSHttpTxnArgSet(txn, config->txn_slot, txnd); TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE); TSDebug(DEBUG_TAG, "Read Req Handler Finished"); @@ -153,21 +134,16 @@ handle_post_remap(TSCont cont, TSEvent event ATS_UNUSED, void *edata) { TSHttpTxn txn = (TSHttpTxn) edata; config_t *config; - txn_data_t *txnd; + void *txnd = (void *) 0x01; // low bit 1 because we are post remap and thus success config = (config_t *) TSContDataGet(cont); if (config->post_remap_host) - { - txnd = TSmalloc(sizeof(txn_data_t)); - txnd->remap_success = true; - txnd->hostname = NULL; - TSHttpTxnArgSet(txn, config->txn_slot, (void *) txnd); - } + TSHttpTxnArgSet(txn, config->txn_slot, txnd); else { - txnd = (txn_data_t *) TSHttpTxnArgGet(txn, config->txn_slot); - txnd->remap_success = true; + txnd = (void *) ((intptr_t)txnd | (intptr_t)TSHttpTxnArgGet(txn, config->txn_slot)); // We need the hostname pre-remap + TSHttpTxnArgSet(txn, config->txn_slot, txnd); } TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE); @@ -175,46 +151,50 @@ handle_post_remap(TSCont cont, TSEvent event ATS_UNUSED, void *edata) return 0; } +#define CREATE_STAT_NAME(s,h,b) snprintf(s, MAX_STAT_LENGTH, "plugin.%s.%s.%s", PLUGIN_NAME, h, b); + static int handle_txn_close(TSCont cont, TSEvent event ATS_UNUSED, void *edata) { TSHttpTxn txn = (TSHttpTxn) edata; config_t *config; - txn_data_t *txnd; + void *txnd; TSHttpStatus status_code = 0; TSMBuffer buf; TSMLoc hdr_loc; uint64_t out_bytes, in_bytes; - char *remap, *stat_name; + char *remap, *hostname; + char *unknown = "unknown"; + char stat_name[MAX_STAT_LENGTH]; config = (config_t *) TSContDataGet(cont); - txnd = (txn_data_t *) TSHttpTxnArgGet(txn, config->txn_slot); + txnd = TSHttpTxnArgGet(txn, config->txn_slot); + + hostname = (char *) ((intptr_t)txnd & (~((intptr_t) 0x01))); // Get hostname if (txnd) { - if (txnd->remap_success) + if ((intptr_t) txnd & 0x01) // remap succeeded? { if (!config->post_remap_host) - remap = txnd->hostname; + remap = hostname; else remap = get_effective_host(txn); if (!remap) - remap = TSstrdup("unknown"); + remap = unknown; in_bytes = TSHttpTxnClientReqHdrBytesGet(txn); in_bytes += TSHttpTxnClientReqBodyBytesGet(txn); - stat_name = create_stat_name(remap, "in_bytes"); + CREATE_STAT_NAME(stat_name, remap, "in_bytes") stat_add(stat_name, (TSMgmtInt) in_bytes, config->persist_type, config->stat_creation_mutex); - TSfree(stat_name); out_bytes = TSHttpTxnClientRespHdrBytesGet(txn); out_bytes += TSHttpTxnClientRespBodyBytesGet(txn); - stat_name = create_stat_name(remap, "out_bytes"); + CREATE_STAT_NAME(stat_name, remap, "out_bytes") stat_add(stat_name, (TSMgmtInt) out_bytes, config->persist_type, config->stat_creation_mutex); - TSfree(stat_name); if (TSHttpTxnClientRespGet(txn, &buf, &hdr_loc) == TS_SUCCESS) { @@ -222,33 +202,29 @@ handle_txn_close(TSCont cont, TSEvent event ATS_UNUSED, void *edata) TSHandleMLocRelease(buf, TS_NULL_MLOC, hdr_loc); if ((status_code >= 200) && (status_code <= 299)) - stat_name = create_stat_name(remap, "status_2xx"); + CREATE_STAT_NAME(stat_name, remap, "status_2xx") else if ((status_code >= 300) && (status_code <= 399)) - stat_name = create_stat_name(remap, "status_3xx"); + CREATE_STAT_NAME(stat_name, remap, "status_3xx") else if ((status_code >= 400) && (status_code <= 499)) - stat_name = create_stat_name(remap, "status_4xx"); + CREATE_STAT_NAME(stat_name, remap, "status_4xx") else if ((status_code >= 500) && ((int)status_code <= 599)) - stat_name = create_stat_name(remap, "status_5xx"); + CREATE_STAT_NAME(stat_name, remap, "status_5xx") else - stat_name = create_stat_name(remap, "status_other"); + CREATE_STAT_NAME(stat_name, remap, "status_other") stat_add(stat_name, 1, config->persist_type, config->stat_creation_mutex); - TSfree(stat_name); } else { - stat_name = create_stat_name(remap, "status_unknown"); + CREATE_STAT_NAME(stat_name, remap, "status_unknown") stat_add(stat_name, 1, config->persist_type, config->stat_creation_mutex); - TSfree(stat_name); } - TSfree(remap); - + if (remap != unknown) + TSfree(remap); } - else if (txnd->hostname) - TSfree(txnd->hostname); - - TSfree(txnd); + else if (hostname) + TSfree(hostname); } TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
