From da84b9c47aa24de241f5611648024c2fb6c81de4 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 10 Jan 2017 11:32:52 +0100 Subject: [PATCH] Defrag: fix comments & code to conform to the Redis code base. Don't go over 80 cols. Start with captial letter, capital letter afer point, end comment with a point and so forth. No actual code behavior touched at all. --- src/defrag.c | 138 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 84 insertions(+), 54 deletions(-) diff --git a/src/defrag.c b/src/defrag.c index d96e5db57..ddfd6e2dd 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -58,14 +58,16 @@ void* activeDefragAlloc(void *ptr) { server.stat_active_defrag_misses++; return NULL; } - /* if this run is more utilized than the average utilization in this bin (or it is full), skip it. - * this will eventually move all the allocations from relatively empty runs into relatively full runs. */ + /* if this run is more utilized than the average utilization in this bin + * (or it is full), skip it. This will eventually move all the allocations + * from relatively empty runs into relatively full runs. */ if (run_util > bin_util || run_util == 1<<16) { server.stat_active_defrag_misses++; return NULL; } /* move this allocation to a new allocation. - * make sure not to use the thread cache. so that we don't get back the same pointers we try to free */ + * make sure not to use the thread cache. so that we don't get back the same + * pointers we try to free */ size = zmalloc_size(ptr); newptr = zmalloc_no_tcache(size); memcpy(newptr, ptr, size); @@ -99,7 +101,7 @@ robj *activeDefragStringOb(robj* ob, int *defragged) { if (ob->refcount!=1) return NULL; - /* try to defrag robj (only if not an EMBSTR type (handled below) */ + /* try to defrag robj (only if not an EMBSTR type (handled below). */ if (ob->type!=OBJ_STRING || ob->encoding!=OBJ_ENCODING_EMBSTR) { if ((ret = activeDefragAlloc(ob))) { ob = ret; @@ -116,7 +118,8 @@ robj *activeDefragStringOb(robj* ob, int *defragged) { (*defragged)++; } } else if (ob->encoding==OBJ_ENCODING_EMBSTR) { - /* the sds is embedded in the object allocation, calculate the offset and update the pointer in the new allocation */ + /* The sds is embedded in the object allocation, calculate the + * offset and update the pointer in the new allocation. */ long ofs = (intptr_t)ob->ptr - (intptr_t)ob; if ((ret = activeDefragAlloc(ob))) { ret->ptr = (void*)((intptr_t)ret + ofs); @@ -129,14 +132,16 @@ robj *activeDefragStringOb(robj* ob, int *defragged) { return ret; } -/* Defrag helper for dictEntries to be used during dict iteration (called on each step). - * returns a stat of how many pointers were moved. */ +/* Defrag helper for dictEntries to be used during dict iteration (called on + * each step). Teturns a stat of how many pointers were moved. */ int dictIterDefragEntry(dictIterator *iter) { - /* This function is a little bit dirty since it messes with the internals of the dict and it's iterator, - * but the benefit is that it is very easy to use, and require no other chagnes in the dict. */ + /* This function is a little bit dirty since it messes with the internals + * of the dict and it's iterator, but the benefit is that it is very easy + * to use, and require no other chagnes in the dict. */ int defragged = 0; dictht *ht; - /* handle the next entry (if there is one), and update the pointer in the current entry. */ + /* Handle the next entry (if there is one), and update the pointer in the + * current entry. */ if (iter->nextEntry) { dictEntry *newde = activeDefragAlloc(iter->nextEntry); if (newde) { @@ -159,8 +164,8 @@ int dictIterDefragEntry(dictIterator *iter) { } /* Defrag helper for dict main allocations (dict struct, and hash tables). - * receives a pointer to the dict* and implicitly updates it when the dict struct itself was moved. - * returns a stat of how many pointers were moved. */ + * receives a pointer to the dict* and implicitly updates it when the dict + * struct itself was moved. Returns a stat of how many pointers were moved. */ int dictDefragTables(dict** dictRef) { dict *d = *dictRef; dictEntry **newtable; @@ -200,10 +205,12 @@ void zslUpdateNode(zskiplist *zsl, zskiplistNode *oldnode, zskiplistNode *newnod } /* Defrag helper for sorted set. - * Update the robj pointer, defrag the skiplist struct and return the new score reference. - * we may not access oldele pointer (not even the pointer stored in the skiplist), as it was already freed. - * newele may be null, in which case we only need to defrag the skiplist, but not update the obj pointer. - * when return value is non-NULL, it is the score reference that must be updated in the dict record. */ + * Update the robj pointer, defrag the skiplist struct and return the new score + * reference. We may not access oldele pointer (not even the pointer stored in + * the skiplist), as it was already freed. Newele may be null, in which case we + * only need to defrag the skiplist, but not update the obj pointer. + * When return value is non-NULL, it is the score reference that must be updated + * in the dict record. */ double *zslDefrag(zskiplist *zsl, double score, sds oldele, sds newele) { zskiplistNode *update[ZSKIPLIST_MAXLEVEL], *x, *newx; int i; @@ -214,7 +221,9 @@ double *zslDefrag(zskiplist *zsl, double score, sds oldele, sds newele) { x = zsl->header; for (i = zsl->level-1; i >= 0; i--) { while (x->level[i].forward && - x->level[i].forward->ele != oldele && /* make sure not to access the ->obj pointer if it matches oldele */ + x->level[i].forward->ele != oldele && /* make sure not to access the + ->obj pointer if it matches + oldele */ (x->level[i].forward->score < score || (x->level[i].forward->score == score && sdscmp(x->level[i].forward->ele,ele) < 0))) @@ -237,12 +246,13 @@ double *zslDefrag(zskiplist *zsl, double score, sds oldele, sds newele) { return NULL; } -/* Utility function that replaces an old key pointer in the dictionary with a new pointer. - * Additionally, we try to defrag the dictEntry in that dict. - * oldkey mey be a dead pointer and should not be accessed (we get a pre-calculated hash value). - * newkey may be null if the key pointer wasn't moved. - * return value is the the dictEntry if found, or NULL if not found. - * NOTE: this is very ugly code, but it let's us avoid the complication of doing a scan on another dict. */ +/* Utility function that replaces an old key pointer in the dictionary with a + * new pointer. Additionally, we try to defrag the dictEntry in that dict. + * Oldkey mey be a dead pointer and should not be accessed (we get a + * pre-calculated hash value). Newkey may be null if the key pointer wasn't + * moved. Return value is the the dictEntry if found, or NULL if not found. + * NOTE: this is very ugly code, but it let's us avoid the complication of + * doing a scan on another dict. */ dictEntry* replaceSateliteDictKeyPtrAndOrDefragDictEntry(dict *d, sds oldkey, sds newkey, unsigned int hash, int *defragged) { dictEntry **deref = dictFindEntryRefByPtrAndHash(d, oldkey, hash); if (deref) { @@ -259,8 +269,9 @@ dictEntry* replaceSateliteDictKeyPtrAndOrDefragDictEntry(dict *d, sds oldkey, sd return NULL; } -/* for each key we scan in the main dict, this function will attempt to defrag all the various pointers it has. - * returns a stat of how many pointers were moved. */ +/* for each key we scan in the main dict, this function will attempt to defrag + * all the various pointers it has. Returns a stat of how many pointers were + * moved. */ int defargKey(redisDb *db, dictEntry *de) { sds keysds = dictGetKey(de); robj *newob, *ob; @@ -270,19 +281,19 @@ int defargKey(redisDb *db, dictEntry *de) { int defragged = 0; sds newsds; - /* try to defrag the key name */ + /* Try to defrag the key name. */ newsds = activeDefragSds(keysds); if (newsds) defragged++, de->key = newsds; if (dictSize(db->expires)) { /* Dirty code: - * i can't search in db->expires for that key after i already released the pointer it holds - * it won't be able to do the string compare */ + * I can't search in db->expires for that key after i already released + * the pointer it holds it won't be able to do the string compare */ unsigned int hash = dictGetHash(db->dict, de->key); replaceSateliteDictKeyPtrAndOrDefragDictEntry(db->expires, keysds, newsds, hash, &defragged); } - /* try to defrag robj and / or string value */ + /* Try to defrag robj and / or string value. */ ob = dictGetVal(de); if ((newob = activeDefragStringOb(ob, &defragged))) { de->v.val = newob; @@ -290,7 +301,7 @@ int defargKey(redisDb *db, dictEntry *de) { } if (ob->type == OBJ_STRING) { - /* already handled in activeDefragStringOb */ + /* Already handled in activeDefragStringOb. */ } else if (ob->type == OBJ_LIST) { if (ob->encoding == OBJ_ENCODING_QUICKLIST) { quicklist *ql = ob->ptr, *newql; @@ -400,7 +411,7 @@ int defargKey(redisDb *db, dictEntry *de) { return defragged; } -/* defrag scan callback for the main db dictionary */ +/* Defrag scan callback for the main db dictionary. */ void defragScanCallback(void *privdata, const dictEntry *de) { int defragged = defargKey((redisDb*)privdata, (dictEntry*)de); server.stat_active_defrag_hits += defragged; @@ -410,8 +421,8 @@ void defragScanCallback(void *privdata, const dictEntry *de) { server.stat_active_defrag_key_misses++; } -/* defrag scan callback for for each hash table bicket, - * used in order to defrag the dictEntry allocations */ +/* Defrag scan callback for for each hash table bicket, + * used in order to defrag the dictEntry allocations. */ void defragDictBucketCallback(void *privdata, dictEntry **bucketref) { UNUSED(privdata); while(*bucketref) { @@ -424,15 +435,24 @@ void defragDictBucketCallback(void *privdata, dictEntry **bucketref) { } /* Utility function to get the fragmentation ratio from jemalloc. - * it is critical to do that by comparing only heap maps that belown to jemalloc, and skip ones the jemalloc keeps as spare. - * since we use this fragmentation ratio in order to decide if a defrag action should be taken or not, - * a false detection can cause the defragmenter to waste a lot of CPU without the possibility of getting any results. */ + * It is critical to do that by comparing only heap maps that belown to + * jemalloc, and skip ones the jemalloc keeps as spare. Since we use this + * fragmentation ratio in order to decide if a defrag action should be taken + * or not, a false detection can cause the defragmenter to waste a lot of CPU + * without the possibility of getting any results. */ float getAllocatorFragmentation(size_t *out_frag_bytes) { size_t epoch = 1, allocated = 0, resident = 0, active = 0, sz = sizeof(size_t); - je_mallctl("epoch", &epoch, &sz, &epoch, sz); /* Update the statistics cached by mallctl. */ - je_mallctl("stats.resident", &resident, &sz, NULL, 0); /* unlike RSS, this does not include RSS from shared libraries and other non heap mappings */ - je_mallctl("stats.active", &active, &sz, NULL, 0); /* unlike resident, this doesn't not include the pages jemalloc reserves for re-use (purge will clean that) */ - je_mallctl("stats.allocated", &allocated, &sz, NULL, 0); /* unlike zmalloc_used_memory, this matches the stats.resident by taking into account all allocations done by this process (not only zmalloc) */ + /* Update the statistics cached by mallctl. */ + je_mallctl("epoch", &epoch, &sz, &epoch, sz); + /* Unlike RSS, this does not include RSS from shared libraries and other non + * heap mappings. */ + je_mallctl("stats.resident", &resident, &sz, NULL, 0); + /* Unlike resident, this doesn't not include the pages jemalloc reserves + * for re-use (purge will clean that). */ + je_mallctl("stats.active", &active, &sz, NULL, 0); + /* Unlike zmalloc_used_memory, this matches the stats.resident by taking + * into account all allocations done by this process (not only zmalloc). */ + je_mallctl("stats.allocated", &allocated, &sz, NULL, 0); float frag_pct = ((float)active / allocated)*100 - 100; size_t frag_bytes = active - allocated; float rss_pct = ((float)resident / allocated)*100 - 100; @@ -461,24 +481,33 @@ void activeDefragCycle(void) { long long start, timelimit; if (server.aof_child_pid!=-1 || server.rdb_child_pid!=-1) - return; /* defragging memory while there's a fork will just do damage. */ + return; /* Defragging memory while there's a fork will just do damage. */ - /* once a second, check if we the fragmentation justfies starting a scan or making it more aggressive */ + /* Once a second, check if we the fragmentation justfies starting a scan + * or making it more aggressive. */ run_with_period(1000) { size_t frag_bytes; float frag_pct = getAllocatorFragmentation(&frag_bytes); - /* if we're not already running, and below the threshold, exit. */ + /* If we're not already running, and below the threshold, exit. */ if (!server.active_defrag_running) { if(frag_pct < server.active_defrag_threshold_lower || frag_bytes < server.active_defrag_ignore_bytes) return; } - /* calculate the adaptive aggressiveness of the defrag */ - int cpu_pct = INTERPOLATE(frag_pct, server.active_defrag_threshold_lower, server.active_defrag_threshold_upper, - server.active_defrag_cycle_min, server.active_defrag_cycle_max); - cpu_pct = LIMIT(cpu_pct, server.active_defrag_cycle_min, server.active_defrag_cycle_max); - /* we allow increasing the aggressiveness during a scan, but don't reduce it */ - if (!server.active_defrag_running || cpu_pct > server.active_defrag_running) { + /* Calculate the adaptive aggressiveness of the defrag */ + int cpu_pct = INTERPOLATE(frag_pct, + server.active_defrag_threshold_lower, + server.active_defrag_threshold_upper, + server.active_defrag_cycle_min, + server.active_defrag_cycle_max); + cpu_pct = LIMIT(cpu_pct, + server.active_defrag_cycle_min, + server.active_defrag_cycle_max); + /* We allow increasing the aggressiveness during a scan, but don't + * reduce it. */ + if (!server.active_defrag_running || + cpu_pct > server.active_defrag_running) + { server.active_defrag_running = cpu_pct; serverLog(LL_VERBOSE, "Starting active defrag, frag=%.0f%%, frag_bytes=%zu, cpu=%d%%", @@ -495,7 +524,7 @@ void activeDefragCycle(void) { do { if (!cursor) { - /* Move on to next database, and stop if we reached the last one */ + /* Move on to next database, and stop if we reached the last one. */ if (++current_db >= server.dbnum) { long long now = ustime(); size_t frag_bytes; @@ -512,7 +541,7 @@ void activeDefragCycle(void) { return; } else if (current_db==0) { - /* start a scan from the first database */ + /* Start a scan from the first database. */ start_scan = ustime(); start_stat = server.stat_active_defrag_hits; } @@ -523,8 +552,9 @@ void activeDefragCycle(void) { do { cursor = dictScan(db->dict, cursor, defragScanCallback, defragDictBucketCallback, db); - /* once in 16 scan iterations, or 1000 pointer reallocations (if we have a lot of pointers in one hash bucket), - * check if we reached the tiem limit */ + /* Once in 16 scan iterations, or 1000 pointer reallocations + * (if we have a lot of pointers in one hash bucket), check if we + * reached the tiem limit. */ if (cursor && (++iterations > 16 || server.stat_active_defrag_hits - defragged > 1000)) { if ((ustime() - start) > timelimit) { return; @@ -539,7 +569,7 @@ void activeDefragCycle(void) { #else /* HAVE_DEFRAG */ void activeDefragCycle(void) { - /* not implemented yet*/ + /* Not implemented yet. */ } #endif