From 4ac19a99fc5ddedff89031edc9ff8e2d7ac80925 Mon Sep 17 00:00:00 2001 From: Guido Reina Date: Sun, 5 May 2013 16:12:17 +0200 Subject: [PATCH 1/4] Typos. --- arangod/PriorityQueue/priorityqueue.h | 4 ++-- lib/BasicsC/associative.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arangod/PriorityQueue/priorityqueue.h b/arangod/PriorityQueue/priorityqueue.h index bfd2e9e853..7c43085084 100644 --- a/arangod/PriorityQueue/priorityqueue.h +++ b/arangod/PriorityQueue/priorityqueue.h @@ -102,7 +102,7 @@ typedef struct TRI_pqueue_base_s { bool _reverse; // ........................................................................... - // Additional hidden extenral structure used outside this priority queue + // Additional hidden external structure used outside this priority queue // This hidden structure is not available within this priority queue // ........................................................................... // char[n] @@ -124,7 +124,7 @@ typedef struct TRI_pqueue_s { // ........................................................................... - // default pq add, remove ,top methods + // default pq add, remove, top methods // ........................................................................... bool (*add) (struct TRI_pqueue_s*, void*); diff --git a/lib/BasicsC/associative.h b/lib/BasicsC/associative.h index c07fbea121..d910818cd0 100644 --- a/lib/BasicsC/associative.h +++ b/lib/BasicsC/associative.h @@ -374,7 +374,7 @@ size_t TRI_GetLengthAssociativePointer (const TRI_associative_pointer_t* const); //////////////////////////////////////////////////////////////////////////////// /// @brief associative array of synced pointers /// -/// Note that lookup, insert, and remove are proctected using a read-write lock. +/// Note that lookup, insert, and remove are protected using a read-write lock. //////////////////////////////////////////////////////////////////////////////// typedef struct TRI_associative_synced_s { From 0a08833b0f3f8d59937550e56083a16e2cacf3cc Mon Sep 17 00:00:00 2001 From: Guido Reina Date: Sun, 5 May 2013 16:15:23 +0200 Subject: [PATCH 2/4] In the function PQueueIndex_new(), if TRI_Allocate() failed to allocate memory for the associative array idx->_aa, idx was freed before idx->_pq. Fixed typos. --- arangod/PriorityQueue/pqueueindex.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arangod/PriorityQueue/pqueueindex.c b/arangod/PriorityQueue/pqueueindex.c index 06ce65e696..b40d7ca9ba 100644 --- a/arangod/PriorityQueue/pqueueindex.c +++ b/arangod/PriorityQueue/pqueueindex.c @@ -123,7 +123,7 @@ PQIndex* PQueueIndex_new (void) { bool ok; // .......................................................................... - // Allocate the Priority Que Index + // Allocate the Priority Queue Index // .......................................................................... idx = TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(PQIndex), false); @@ -136,7 +136,7 @@ PQIndex* PQueueIndex_new (void) { // .......................................................................... - // Allocate the priority que + // Allocate the priority queue // Remember to add any additional structure you need // .......................................................................... @@ -155,8 +155,8 @@ PQIndex* PQueueIndex_new (void) { idx->_aa = TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_associative_array_t), false); if (idx->_aa == NULL) { - TRI_Free(TRI_UNKNOWN_MEM_ZONE, idx); TRI_Free(TRI_UNKNOWN_MEM_ZONE, idx->_pq); + TRI_Free(TRI_UNKNOWN_MEM_ZONE, idx); TRI_set_errno(TRI_ERROR_OUT_OF_MEMORY); LOG_ERROR("out of memory when creating priority queue index"); return NULL; @@ -164,7 +164,7 @@ PQIndex* PQueueIndex_new (void) { // .......................................................................... - // Initialise the priority que + // Initialise the priority queue // .......................................................................... ok = TRI_InitPQueue(idx->_pq, From 105662d5ba12eb942d65c05c542aaba91b0f2550 Mon Sep 17 00:00:00 2001 From: Guido Reina Date: Sun, 5 May 2013 16:33:09 +0200 Subject: [PATCH 3/4] [Small improvements] In the function TRI_InitPQueue(), instead of initializing pq->_base._items with '\0's by using memset(), TRI_Allocate() is called with 'set' set to true (as it is done in the function CheckPQSize()). In the function CheckPQSize(), the priority queue capacity will be increased only when the new item to be inserted doesn't fit (pq->_base._count + 1 > pq->_base._capacity). In the function CheckPQSize(), instead of allocating a new buffer and then copying the elements, TRI_Reallocate() is used and the new area is filled with '\0's. --- arangod/PriorityQueue/priorityqueue.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/arangod/PriorityQueue/priorityqueue.c b/arangod/PriorityQueue/priorityqueue.c index dc8e7db3e8..355b409309 100644 --- a/arangod/PriorityQueue/priorityqueue.c +++ b/arangod/PriorityQueue/priorityqueue.c @@ -126,22 +126,16 @@ bool TRI_InitPQueue (TRI_pqueue_t* pq, size_t initialCapacity, size_t itemSize, // .......................................................................... - // Set the capacity and assign memeory for storage + // Set the capacity and assign memory for storage // .......................................................................... pq->_base._capacity = initialCapacity; - pq->_base._items = TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, pq->_base._itemSize * pq->_base._capacity, false); + pq->_base._items = TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, pq->_base._itemSize * pq->_base._capacity, true); if (pq->_base._items == NULL) { TRI_set_errno(TRI_ERROR_OUT_OF_MEMORY); LOG_ERROR("out of memory when creating priority queue storage"); return false; } - // .......................................................................... - // Initialise the memory allcoated for the storage of pq (0 filled) - // .......................................................................... - - memset(pq->_base._items, 0, pq->_base._itemSize * pq->_base._capacity); - // .......................................................................... // initialise the number of items stored @@ -373,26 +367,29 @@ static void* TopPQueue(TRI_pqueue_t* pq) { static bool CheckPQSize(TRI_pqueue_t* pq) { char* newItems; + size_t newCapacity; if (pq == NULL) { return false; } - if (pq->_base._capacity > (pq->_base._count + 1) ) { + if (pq->_base._capacity >= (pq->_base._count + 1) ) { return true; } - pq->_base._capacity = pq->_base._capacity * 2; - // allocate and fill with NUL bytes - newItems = TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, pq->_base._capacity * pq->_base._itemSize, true); + newCapacity = pq->_base._capacity * 2; + + // reallocate + newItems = TRI_Reallocate(TRI_UNKNOWN_MEM_ZONE, pq->_base._items, newCapacity * pq->_base._itemSize); if (newItems == NULL) { return false; } - memcpy(newItems, pq->_base._items, (pq->_base._count * pq->_base._itemSize) ); + // initialise the remaining memory allocated for the storage of pq (0 filled) + memset(pq->_base._items + pq->_base._capacity * pq->_base._itemSize, 0, (newCapacity - pq->_base._capacity) * pq->_base._itemSize); - TRI_Free(TRI_UNKNOWN_MEM_ZONE, pq->_base._items); pq->_base._items = newItems; + pq->_base._capacity = newCapacity; return true; } From 2f4319b31a711b07a5d7c8ce5686d69856dc165b Mon Sep 17 00:00:00 2001 From: Guido Reina Date: Sun, 5 May 2013 18:10:37 +0200 Subject: [PATCH 4/4] [Small improvements] In the function ResizeAssociativeArray(), array->_table is already initialized to '\0's, so there is no need to call clearElement() for each element. An associative array is only used by PQIndex and MasterTable_t. For PQIndex the function ClearElementPQIndex() will be used, which sets the whole element to '\0's. For MasterTable_t the function tablePositionClearElement() will be used, which sets all but one field to 0/NULL (the only field which is not set is _vectorNum). If the clearElement() should be called, maybe TRI_Allocate() could be called with false, to avoid double initialization. When copying elements, the for loop stops when the number of elements is the same as the original number of elements. In the function TRI_FindByKeyAssociativeArray(), there is no need to check whether the element is not empty and is equal key element, only if it is not empty (as it is done in the function TRI_InsertKeyAssociativeArray()). In the function TRI_FindByElementAssociativeArray(), there is no need to check whether the element is not empty and is equal element element, only if it is not empty (as it is done in the function TRI_InsertElementAssociativeArray()). --- lib/BasicsC/associative.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/BasicsC/associative.c b/lib/BasicsC/associative.c index c45a3f3ee7..9882013b2c 100644 --- a/lib/BasicsC/associative.c +++ b/lib/BasicsC/associative.c @@ -77,6 +77,7 @@ static void AddNewElement (TRI_associative_array_t* array, void* element) { static void ResizeAssociativeArray (TRI_associative_array_t* array) { char * oldTable; uint32_t oldAlloc; + uint32_t oldUsed; uint32_t j; oldTable = array->_table; @@ -96,13 +97,10 @@ static void ResizeAssociativeArray (TRI_associative_array_t* array) { return; } + oldUsed = array->_nrUsed; array->_nrUsed = 0; - for (j = 0; j < array->_nrAlloc; j++) { - array->clearElement(array, array->_table + j * array->_elementSize); - } - - for (j = 0; j < oldAlloc; j++) { + for (j = 0; array->_nrUsed < oldUsed; j++) { if (! array->isEmptyElement(array, oldTable + j * array->_elementSize)) { AddNewElement(array, oldTable + j * array->_elementSize); } @@ -249,7 +247,7 @@ void* TRI_FindByKeyAssociativeArray (TRI_associative_array_t* array, void* key) element = TRI_LookupByKeyAssociativeArray(array, key); - if (! array->isEmptyElement(array, element) && array->isEqualKeyElement(array, key, element)) { + if (! array->isEmptyElement(array, element)) { return element; } @@ -295,7 +293,7 @@ void* TRI_FindByElementAssociativeArray (TRI_associative_array_t* array, void* e element2 = TRI_LookupByElementAssociativeArray(array, element); - if (! array->isEmptyElement(array, element2) && array->isEqualElementElement(array, element2, element)) { + if (! array->isEmptyElement(array, element2)) { return element2; }