From 770b65de804fc56a3e6a00d3916958c99774880d Mon Sep 17 00:00:00 2001 From: David Steele Date: Mon, 26 Oct 2020 12:18:45 -0400 Subject: [PATCH] Improve performance of large file lists in backup/restore commands. lstRemoveIdx(list, 0) resulted in the entire list being moved down to the first position which could take a long time for big lists. This is a common pattern in backup/restore when processing file queues. Instead simply move the list pointer up when first item is removed. Then on insert check if there is space at the beginning when there is no longer space at the end and do the move then. This way if a list is built and then drained without any new inserts then no move is required. --- doc/xml/release.xml | 12 ++++++++++ src/common/type/list.c | 33 +++++++++++++++++++++----- test/define.yaml | 2 +- test/src/module/common/typeListTest.c | 16 ++++++++++++- test/src/module/performance/typeTest.c | 27 +++++++++++++++++++++ 5 files changed, 82 insertions(+), 8 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index cf87a803c..cb075e08f 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -49,6 +49,18 @@ + + + + + + + + +

Improve performance of large file lists in backup/restore commands.

+ + + diff --git a/src/common/type/list.c b/src/common/type/list.c index ba6e41841..b419c206e 100644 --- a/src/common/type/list.c +++ b/src/common/type/list.c @@ -22,7 +22,8 @@ struct List unsigned int listSize; unsigned int listSizeMax; SortOrder sortOrder; - unsigned char *list; + unsigned char *listAlloc; // Pointer to memory allocated for the list + unsigned char *list; // Pointer to the current start of the list ListComparator *comparator; }; @@ -294,13 +295,24 @@ lstInsert(List *this, unsigned int listIdx, const void *item) this->listSizeMax *= 2; this->list = memResize(this->list, this->listSizeMax * this->itemSize); } + + this->listAlloc = this->list; } MEM_CONTEXT_END(); } + // Else if there is space before the beginning of the list then move the list down + else if ( + (this->list != this->listAlloc) && + (this->listSize + ((uintptr_t)(this->list - this->listAlloc) / this->itemSize) == this->listSizeMax)) + { + memmove(this->listAlloc, this->list, this->listSize * this->itemSize); + this->list = this->listAlloc; + } - // If not inserting at the end then move items down to make space + // Calculate the position where this item will be copied void *itemPtr = this->list + (listIdx * this->itemSize); + // If not inserting at the end then move items down to make space if (listIdx != lstSize(this)) memmove(this->list + ((listIdx + 1) * this->itemSize), itemPtr, (lstSize(this) - listIdx) * this->itemSize); @@ -324,12 +336,21 @@ lstRemoveIdx(List *this, unsigned int listIdx) ASSERT(this != NULL); ASSERT(listIdx <= lstSize(this)); - // Remove the item by moving the items after it down + // Decrement the list size this->listSize--; - memmove( - this->list + (listIdx * this->itemSize), this->list + ((listIdx + 1) * this->itemSize), - (lstSize(this) - listIdx) * this->itemSize); + // If this is the first item then move the list pointer up to avoid moving all the items + if (listIdx == 0) + { + this->list += this->itemSize; + } + // Else remove the item by moving the items after it down + else + { + memmove( + this->list + (listIdx * this->itemSize), this->list + ((listIdx + 1) * this->itemSize), + (lstSize(this) - listIdx) * this->itemSize); + } FUNCTION_TEST_RETURN(this); } diff --git a/test/define.yaml b/test/define.yaml index ecef28545..263fad38f 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -791,7 +791,7 @@ performance: test: # ---------------------------------------------------------------------------------------------------------------------------- - name: type - total: 4 + total: 5 # ---------------------------------------------------------------------------------------------------------------------------- - name: storage diff --git a/test/src/module/common/typeListTest.c b/test/src/module/common/typeListTest.c index 77575176a..a68233220 100644 --- a/test/src/module/common/typeListTest.c +++ b/test/src/module/common/typeListTest.c @@ -115,6 +115,20 @@ testRun(void) TEST_RESULT_INT(*item, listIdx + 1, "check item %u", listIdx); } + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("add items to force the list to get moved down"); + + TEST_RESULT_INT(lstSize(list), 8, "check size"); + TEST_RESULT_INT(list->listSizeMax, 16, "check size max"); + + for (int listIdx = 0; listIdx < 8; listIdx++) + { + int item = listIdx + 9; + TEST_RESULT_VOID(lstAdd(list, &item), "add item %d", item); + } + + TEST_RESULT_INT(list->listSize, list->listSizeMax, "size equals max size"); + // Remove last item TEST_RESULT_VOID(lstRemoveLast(list), "remove last item"); @@ -125,7 +139,7 @@ testRun(void) TEST_RESULT_INT(*item, listIdx + 1, "check item %u", listIdx); } - TEST_ERROR(lstGet(list, lstSize(list)), AssertError, "cannot get index 7 from list with 7 value(s)"); + TEST_ERROR(lstGet(list, lstSize(list)), AssertError, "cannot get index 15 from list with 15 value(s)"); TEST_RESULT_VOID(lstMove(NULL, memContextTop()), "move null list"); } diff --git a/test/src/module/performance/typeTest.c b/test/src/module/performance/typeTest.c index 003ed337c..2b1645690 100644 --- a/test/src/module/performance/typeTest.c +++ b/test/src/module/performance/typeTest.c @@ -187,6 +187,33 @@ testRun(void) TEST_LOG_FMT("desc search completed in %ums", (unsigned int)(timeMSec() - timeBegin)); } + // ***************************************************************************************************************************** + if (testBegin("lstRemoveIdx()")) + { + CHECK(testScale() <= 10000); + int testMax = 1000000 * (int)testScale(); + + // Generate a large list of values (use int instead of string so there fewer allocations) + List *list = lstNewP(sizeof(int)); + + for (int listIdx = 0; listIdx < testMax; listIdx++) + lstAdd(list, &listIdx); + + CHECK(lstSize(list) == (unsigned int)testMax); + + TEST_LOG_FMT("generated %d item list", testMax); + + // Remove all values from index 0 + TimeMSec timeBegin = timeMSec(); + + for (int listIdx = 0; listIdx < testMax; listIdx++) + lstRemoveIdx(list, 0); + + TEST_LOG_FMT("remove completed in %ums", (unsigned int)(timeMSec() - timeBegin)); + + CHECK(lstSize(list) == 0); + } + // ***************************************************************************************************************************** if (testBegin("iniLoad()")) {