From 4387250f2e159dc4f762ed4a7f4180eb81db3717 Mon Sep 17 00:00:00 2001 From: David Steele Date: Fri, 8 Mar 2024 10:07:03 +1300 Subject: [PATCH] Improve sort comparators. Improve sort comparators to use branchless comparisons when possible and avoid using subtraction. Only one comparator was using subtraction and it appears there was no overflow risk since the values were pretty small. Inspired by https://www.postgresql.org/message-id/CA%2B14426g2Wa9QuUpmakwPxXFWG_1FaY0AsApkvcTBy-YfS6uaw%40mail.gmail.com. --- doc/xml/release/2024/2.51.xml | 13 +++++++++++++ src/command/archive/common.c | 14 ++++++++------ src/command/backup/blockMap.c | 8 +++----- src/common/type/list.c | 19 ++++++++----------- src/common/type/list.h | 4 ++++ test/src/module/common/typeListTest.c | 11 +---------- test/src/module/performance/typeTest.c | 11 +---------- 7 files changed, 38 insertions(+), 42 deletions(-) diff --git a/doc/xml/release/2024/2.51.xml b/doc/xml/release/2024/2.51.xml index 222f22f30..ced19c88e 100644 --- a/doc/xml/release/2024/2.51.xml +++ b/doc/xml/release/2024/2.51.xml @@ -53,6 +53,19 @@

Add detailed backtrace to autoconf/make build.

+ + + + + + + + + + +

Improve sort comparators.

+ + diff --git a/src/command/archive/common.c b/src/command/archive/common.c index e18be6160..4fc1d6c5f 100644 --- a/src/command/archive/common.c +++ b/src/command/archive/common.c @@ -15,6 +15,7 @@ Archive Common #include "common/log.h" #include "common/memContext.h" #include "common/regExp.h" +#include "common/type/convert.h" #include "common/wait.h" #include "config/config.h" #include "postgres/interface.h" @@ -293,14 +294,15 @@ archiveAsyncExec(ArchiveMode archiveMode, const StringList *commandExec) /**********************************************************************************************************************************/ FN_EXTERN int -archiveIdComparator(const void *item1, const void *item2) +archiveIdComparator(const void *const archiveId1, const void *const archiveId2) { - StringList *archiveSort1 = strLstNewSplitZ(*(String **)item1, "-"); - StringList *archiveSort2 = strLstNewSplitZ(*(String **)item2, "-"); - int int1 = atoi(strZ(strLstGet(archiveSort1, 1))); - int int2 = atoi(strZ(strLstGet(archiveSort2, 1))); + ASSERT(strstr(strZ(*(String **)archiveId1), "-") != NULL); + ASSERT(strstr(strZ(*(String **)archiveId2), "-") != NULL); - return int1 - int2; + const int id1 = cvtZToInt(strstr(strZ(*(String **)archiveId1), "-") + 1); + const int id2 = cvtZToInt(strstr(strZ(*(String **)archiveId2), "-") + 1); + + return LST_COMPARATOR_CMP(id1, id2); } /**********************************************************************************************************************************/ diff --git a/src/command/backup/blockMap.c b/src/command/backup/blockMap.c index 24761e5da..4a5fd737d 100644 --- a/src/command/backup/blockMap.c +++ b/src/command/backup/blockMap.c @@ -76,12 +76,10 @@ lstComparatorBlockMapReference(const void *const blockMapRef1, const void *const ASSERT(blockMapRef1 != NULL); ASSERT(blockMapRef2 != NULL); - if (((BlockMapReference *)blockMapRef1)->reference < ((BlockMapReference *)blockMapRef2)->reference) - FUNCTION_TEST_RETURN(INT, -1); - else if (((BlockMapReference *)blockMapRef1)->reference > ((BlockMapReference *)blockMapRef2)->reference) - FUNCTION_TEST_RETURN(INT, 1); + const unsigned int reference1 = ((BlockMapReference *)blockMapRef1)->reference; + const unsigned int reference2 = ((BlockMapReference *)blockMapRef2)->reference; - FUNCTION_TEST_RETURN(INT, 0); + FUNCTION_TEST_RETURN(INT, LST_COMPARATOR_CMP(reference1, reference2)); } FN_EXTERN BlockMap * diff --git a/src/common/type/list.c b/src/common/type/list.c index f4c0241b5..035c86905 100644 --- a/src/common/type/list.c +++ b/src/common/type/list.c @@ -89,23 +89,20 @@ lstComparatorStr(const void *item1, const void *item2) /**********************************************************************************************************************************/ FN_EXTERN int -lstComparatorUInt(const void *const item1, const void *const item2) +lstComparatorUInt(const void *const uintPtr1, const void *const uintPtr2) { FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM_P(VOID, item1); - FUNCTION_TEST_PARAM_P(VOID, item2); + FUNCTION_TEST_PARAM_P(VOID, uintPtr1); + FUNCTION_TEST_PARAM_P(VOID, uintPtr2); FUNCTION_TEST_END(); - ASSERT(item1 != NULL); - ASSERT(item2 != NULL); + ASSERT(uintPtr1 != NULL); + ASSERT(uintPtr2 != NULL); - if (*(unsigned int *)item1 < *(unsigned int *)item2) - FUNCTION_TEST_RETURN(INT, -1); + const unsigned int uint1 = *(unsigned int *)uintPtr1; + const unsigned int uint2 = *(unsigned int *)uintPtr2; - if (*(unsigned int *)item1 > *(unsigned int *)item2) - FUNCTION_TEST_RETURN(INT, 1); - - FUNCTION_TEST_RETURN(INT, 0); + FUNCTION_TEST_RETURN(INT, LST_COMPARATOR_CMP(uint1, uint2)); } /**********************************************************************************************************************************/ diff --git a/src/common/type/list.h b/src/common/type/list.h index 164579149..0015bafff 100644 --- a/src/common/type/list.h +++ b/src/common/type/list.h @@ -52,6 +52,10 @@ FN_EXTERN int lstComparatorUInt(const void *item1, const void *item2); // General purpose list comparator for zero-terminated strings or structs with a zero-terminated string as the first member FN_EXTERN int lstComparatorZ(const void *item1, const void *item2); +// Macro to compare two values in a branchless and transitive fashion +#define LST_COMPARATOR_CMP(item1, item2) \ + (((item1) > (item2)) - ((item1) < (item2))) + /*********************************************************************************************************************************** Constructors ***********************************************************************************************************************************/ diff --git a/test/src/module/common/typeListTest.c b/test/src/module/common/typeListTest.c index eea6c64a6..9692912b9 100644 --- a/test/src/module/common/typeListTest.c +++ b/test/src/module/common/typeListTest.c @@ -9,16 +9,7 @@ Test sort comparator static int testComparator(const void *item1, const void *item2) { - int int1 = *(int *)item1; - int int2 = *(int *)item2; - - if (int1 < int2) - return -1; - - if (int1 > int2) - return 1; - - return 0; + return LST_COMPARATOR_CMP(*(int *)item1, *(int *)item2); } /*********************************************************************************************************************************** diff --git a/test/src/module/performance/typeTest.c b/test/src/module/performance/typeTest.c index b96147cf7..2a54d0929 100644 --- a/test/src/module/performance/typeTest.c +++ b/test/src/module/performance/typeTest.c @@ -31,16 +31,7 @@ Test sort comparator static int testComparator(const void *item1, const void *item2) { - int int1 = *(int *)item1; - int int2 = *(int *)item2; - - if (int1 < int2) - return -1; - - if (int1 > int2) - return 1; - - return 0; + return LST_COMPARATOR_CMP(*(int *)item1, *(int *)item2); } /***********************************************************************************************************************************