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

@@ -53,6 +53,19 @@
                 <p>Add detailed backtrace to <proper>autoconf</proper>/<proper>make</proper> build.</p>
             </release-item>
         </release-improvement-list>
+
+        <release-development-list>
+            <release-item>
+                <github-pull-request id="2280"/>
+
+                <release-item-contributor-list>
+                    <release-item-contributor id="david.steele"/>
+                    <release-item-reviewer id="stephen.frost"/>
+                </release-item-contributor-list>
+
+                <p>Improve sort comparators.</p>
+            </release-item>
+        </release-development-list>
     </release-core-list>
 
     <release-doc-list>

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);
 }
 
 /**********************************************************************************************************************************/

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 *

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));
 }
 
 /**********************************************************************************************************************************/

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
 ***********************************************************************************************************************************/

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);
 }
 
 /***********************************************************************************************************************************

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);
 }
 
 /***********************************************************************************************************************************