On Tue, Oct 6, 2015 at 1:16 PM, Robert Haas <robertmh...@gmail.com> wrote:
> If you would care to revise the patch accordingly, I will commit it
> (barring objections from others, of course).
Here is a revision of 0001-*, with both BSWAP32() and BSWAP64() in a
new header, src/port/pg_bswap.h.
No revisions were required to any other patch in the patch series to
make this work, and so I only include a revised 0001-*.
--
Peter Geoghegan
From f671bbccc1eb2a7ecb5c64925630a5593f888a9d Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <peter.geoghega...@gmail.com>
Date: Mon, 29 Jun 2015 23:53:05 -0400
Subject: [PATCH 1/4] Provide for unsigned comparisons of abbreviated keys
Add ABBREV_STRING_UINT() macro to allow string-like types to represent
abbreviated keys as unsigned integers. ABBREV_STRING_UINT() will use
the new BSWAP64() byte swapping macro (also introduced in this commit)
on 64-bit little-endian platforms, or the existing BSWAP32() macro on
32-bit platforms.
In future commits, certain types with abbreviation support will call
ABBREV_STRING_UINT() to perform a final conversion process on their
string-like abbreviated keys. This makes it safe and portable to
implement the abbreviated key comparator as a simple 3-way unsigned
integer comparator, which must also be changed. By using an
exceptionally cheap abbreviated comparator, a significant boost in sort
performance can be seen for these types on at least some platforms.
---
config/c-compiler.m4 | 18 ++++++++++++++
configure | 24 +++++++++++++++++++
configure.in | 1 +
src/include/pg_config.h.in | 3 +++
src/include/pg_config.h.win32 | 3 +++
src/include/port/pg_bswap.h | 46 +++++++++++++++++++++++++++++++++++
src/include/port/pg_crc32c.h | 12 ++--------
src/include/utils/sortsupport.h | 53 +++++++++++++++++++++++++++++++++++++++++
8 files changed, 150 insertions(+), 10 deletions(-)
create mode 100644 src/include/port/pg_bswap.h
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 9feec0c..550d034 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -214,6 +214,24 @@ fi])# PGAC_C_BUILTIN_BSWAP32
+# PGAC_C_BUILTIN_BSWAP64
+# -------------------------
+# Check if the C compiler understands __builtin_bswap64(),
+# and define HAVE__BUILTIN_BSWAP64 if so.
+AC_DEFUN([PGAC_C_BUILTIN_BSWAP64],
+[AC_CACHE_CHECK(for __builtin_bswap64, pgac_cv__builtin_bswap64,
+[AC_COMPILE_IFELSE([AC_LANG_SOURCE(
+[static unsigned long int x = __builtin_bswap64(0xaabbccddeeff0011);]
+)],
+[pgac_cv__builtin_bswap64=yes],
+[pgac_cv__builtin_bswap64=no])])
+if test x"$pgac_cv__builtin_bswap64" = xyes ; then
+AC_DEFINE(HAVE__BUILTIN_BSWAP64, 1,
+ [Define to 1 if your compiler understands __builtin_bswap64.])
+fi])# PGAC_C_BUILTIN_BSWAP64
+
+
+
# PGAC_C_BUILTIN_CONSTANT_P
# -------------------------
# Check if the C compiler understands __builtin_constant_p(),
diff --git a/configure b/configure
index 99ef10f..b771a83 100755
--- a/configure
+++ b/configure
@@ -11259,6 +11259,30 @@ if test x"$pgac_cv__builtin_bswap32" = xyes ; then
$as_echo "#define HAVE__BUILTIN_BSWAP32 1" >>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_bswap64" >&5
+$as_echo_n "checking for __builtin_bswap64... " >&6; }
+if ${pgac_cv__builtin_bswap64+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+static unsigned long int x = __builtin_bswap64(0xaabbccddeeff0011);
+
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv__builtin_bswap64=yes
+else
+ pgac_cv__builtin_bswap64=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_bswap64" >&5
+$as_echo "$pgac_cv__builtin_bswap64" >&6; }
+if test x"$pgac_cv__builtin_bswap64" = xyes ; then
+
+$as_echo "#define HAVE__BUILTIN_BSWAP64 1" >>confdefs.h
+
+fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_constant_p" >&5
$as_echo_n "checking for __builtin_constant_p... " >&6; }
if ${pgac_cv__builtin_constant_p+:} false; then :
diff --git a/configure.in b/configure.in
index 4143451..b5868b0 100644
--- a/configure.in
+++ b/configure.in
@@ -1317,6 +1317,7 @@ PGAC_C_FUNCNAME_SUPPORT
PGAC_C_STATIC_ASSERT
PGAC_C_TYPES_COMPATIBLE
PGAC_C_BUILTIN_BSWAP32
+PGAC_C_BUILTIN_BSWAP64
PGAC_C_BUILTIN_CONSTANT_P
PGAC_C_BUILTIN_UNREACHABLE
PGAC_C_VA_ARGS
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index dda73a8..a20e0cd 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -660,6 +660,9 @@
/* Define to 1 if your compiler understands __builtin_bswap32. */
#undef HAVE__BUILTIN_BSWAP32
+/* Define to 1 if your compiler understands __builtin_bswap64. */
+#undef HAVE__BUILTIN_BSWAP64
+
/* Define to 1 if your compiler understands __builtin_constant_p. */
#undef HAVE__BUILTIN_CONSTANT_P
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 6f7a773..8566065 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -508,6 +508,9 @@
/* Define to 1 if your compiler understands __builtin_bswap32. */
/* #undef HAVE__BUILTIN_BSWAP32 */
+/* Define to 1 if your compiler understands __builtin_bswap64. */
+/* #undef HAVE__BUILTIN_BSWAP64 */
+
/* Define to 1 if your compiler understands __builtin_constant_p. */
/* #undef HAVE__BUILTIN_CONSTANT_P */
diff --git a/src/include/port/pg_bswap.h b/src/include/port/pg_bswap.h
new file mode 100644
index 0000000..6555942
--- /dev/null
+++ b/src/include/port/pg_bswap.h
@@ -0,0 +1,46 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_bswap.h
+ * Byte swapping.
+ *
+ * Macros for reversing the byte order of 32-bit and 64-bit unsigned integers.
+ * For example, 0xAABBCCDD becomes 0xDDCCBBAA. These are just wrappers for
+ * built-in functions provided by the compiler where support exists.
+ *
+ * Note that the GCC built-in functions __builtin_bswap32() and
+ * __builtin_bswap64() are documented as accepting single arguments of type
+ * uint32_t and uint64_t respectively (these are also the respective return
+ * types). Use caution when using these wrapper macros with signed integers.
+ *
+ * Copyright (c) 2015, PostgreSQL Global Development Group
+ *
+ * src/include/port/pg_bswap.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef PG_BSWAP_H
+#define PG_BSWAP_H
+
+#ifdef HAVE__BUILTIN_BSWAP32
+#define BSWAP32(x) __builtin_bswap32(x)
+#else
+#define BSWAP32(x) (((x << 24) & 0xff000000) | \
+ ((x << 8) & 0x00ff0000) | \
+ ((x >> 8) & 0x0000ff00) | \
+ ((x >> 24) & 0x000000ff))
+#endif /* HAVE__BUILTIN_BSWAP32 */
+
+#ifdef HAVE__BUILTIN_BSWAP64
+#define BSWAP64(x) __builtin_bswap64(x)
+#else
+#define BSWAP64(x) (((x << 56) & 0xff00000000000000UL) | \
+ ((x << 40) & 0x00ff000000000000UL) | \
+ ((x << 24) & 0x0000ff0000000000UL) | \
+ ((x << 8) & 0x000000ff00000000UL) | \
+ ((x >> 8) & 0x00000000ff000000UL) | \
+ ((x >> 24) & 0x0000000000ff0000UL) | \
+ ((x >> 40) & 0x000000000000ff00UL) | \
+ ((x >> 56) & 0x00000000000000ffUL))
+#endif /* HAVE__BUILTIN_BSWAP64 */
+
+#endif /* PG_BSWAP_H */
diff --git a/src/include/port/pg_crc32c.h b/src/include/port/pg_crc32c.h
index c925c56..35589c0 100644
--- a/src/include/port/pg_crc32c.h
+++ b/src/include/port/pg_crc32c.h
@@ -33,6 +33,8 @@
#ifndef PG_CRC32C_H
#define PG_CRC32C_H
+#include "port/pg_bswap.h"
+
typedef uint32 pg_crc32c;
/* The INIT and EQ macros are the same for all implementations. */
@@ -71,16 +73,6 @@ extern pg_crc32c (*pg_comp_crc32c) (pg_crc32c crc, const void *data, size_t len)
#define COMP_CRC32C(crc, data, len) \
((crc) = pg_comp_crc32c_sb8((crc), (data), (len)))
#ifdef WORDS_BIGENDIAN
-
-#ifdef HAVE__BUILTIN_BSWAP32
-#define BSWAP32(x) __builtin_bswap32(x)
-#else
-#define BSWAP32(x) (((x << 24) & 0xff000000) | \
- ((x << 8) & 0x00ff0000) | \
- ((x >> 8) & 0x0000ff00) | \
- ((x >> 24) & 0x000000ff))
-#endif
-
#define FIN_CRC32C(crc) ((crc) = BSWAP32(crc) ^ 0xFFFFFFFF)
#else
#define FIN_CRC32C(crc) ((crc) ^= 0xFFFFFFFF)
diff --git a/src/include/utils/sortsupport.h b/src/include/utils/sortsupport.h
index 4258630..ae1291b 100644
--- a/src/include/utils/sortsupport.h
+++ b/src/include/utils/sortsupport.h
@@ -53,6 +53,7 @@
#define SORTSUPPORT_H
#include "access/attnum.h"
+#include "port/pg_bswap.h"
#include "utils/relcache.h"
typedef struct SortSupportData *SortSupport;
@@ -267,6 +268,58 @@ ApplySortAbbrevFullComparator(Datum datum1, bool isNull1,
return compare;
}
+/*
+ * Make unsigned integer (Datum) 3-way comparisons safe for string-like types.
+ *
+ * For some types, their abbreviated representations could reasonably be built
+ * such that comparisons are binary string comparisons. For example, for a
+ * string type, an abbreviated comparator like this might be used:
+ *
+ * static int
+ * btstringcmp_abbrev(Datum x, Datum y, SortSupport ssup)
+ * {
+ * return memcmp((char *) &x, (char *) &y, sizeof(Datum));
+ * }
+ *
+ * Perhaps the conversion routine packs NUL bytes used as sentinel values
+ * representing termination in the event of short strings too, so short strings
+ * require no special treatment (assuming NUL bytes are generally disallowed).
+ * There might be slightly different specifics for each type, but this basic
+ * pattern recurs relatively often.
+ *
+ * On big-endian machines, compilers might be able to optimize this to just a
+ * few CPU instructions (involving unsigned integer comparisons). Opclass
+ * authors should not rely on that, though. Instead, they should use the
+ * ABBREV_STRING_UINT() macro within their conversion routine to make sure that
+ * it's safe to use a 3-way comparator that performs simple unsigned integer
+ * comparisons explicitly. The abbreviated comparator should then look like
+ * this instead:
+ *
+ * static int
+ * btstringcmp_abbrev(Datum x, Datum y, SortSupport ssup)
+ * {
+ * if (x > y)
+ * return 1;
+ * else if (x == y)
+ * return 0;
+ * else
+ * return -1;
+ * }
+ *
+ * Apart from ensuring even cheaper comparisons on big-endian machines, this
+ * technique also works on little-endian machines, since a byteswap can be
+ * performed within ABBREV_STRING_UINT().
+ */
+#ifdef WORDS_BIGENDIAN
+#define ABBREV_STRING_UINT(x) (x)
+#else /* !WORDS_BIGENDIAN */
+#if SIZEOF_DATUM == 8
+#define ABBREV_STRING_UINT(x) BSWAP64(x)
+#else /* SIZEOF_DATUM != 8 */
+#define ABBREV_STRING_UINT(x) BSWAP32(x)
+#endif /* SIZEOF_DATUM == 8 */
+#endif /* WORDS_BIGENDIAN */
+
/* Other functions in utils/sort/sortsupport.c */
extern void PrepareSortSupportComparisonShim(Oid cmpFunc, SortSupport ssup);
extern void PrepareSortSupportFromOrderingOp(Oid orderingOp, SortSupport ssup);
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
