On Monday 15 November 2010 17:12:25 Robert Haas wrote:> I notice that int8out
isn't terribly consistent with int2out and
> int4out, in that it does an extra copy. Maybe that's justified given
> the greater potential memory wastage, but I'm not certain. One
> approach might be to pick some threshold value and allocate a buffer
> in one of two sizes based on how large the value is relative to that
> cutoff. But that might also be a stupid idea, not sure.
I removed the extra buffer - its actually a tiny bit faster without it (I
guess the allocation pattern is a bit nicer during copy as it will always take
the same paths and eventually the same address).
I couldn't measure any difference memory-usage wise.
The code was that way before btw.
> It would speed things up for me if you or someone else could take a
> quick pass over what remains here and fix the formatting and
> whitespace to be consistent with our general project style, and make
> the comment headers more consistent among the functions being
> added/modified.
I think I did most of those - the function comments in numutils weren't
consistent before - now its consistent with the unchanged pg_atoi.
Thanks for reviewing/applying the first part,
Andres
From 55acfa4f971f5a0e33eb8b9e66d621c16be96d42 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Fri, 19 Nov 2010 21:44:29 +0100
Subject: [PATCH] Implement custom int[248]->string conversion routines out of speed reasons.
---
src/backend/utils/adt/int8.c | 10 +--
src/backend/utils/adt/numutils.c | 130 ++++++++++++++++++++++++++++++++----
src/include/utils/builtins.h | 1 +
src/test/regress/expected/int2.out | 13 ++++
src/test/regress/expected/int4.out | 13 ++++
src/test/regress/expected/int8.out | 13 ++++
src/test/regress/sql/int2.sql | 4 +
src/test/regress/sql/int4.sql | 4 +
src/test/regress/sql/int8.sql | 4 +
9 files changed, 172 insertions(+), 20 deletions(-)
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 894110d..8f4ef5a 100644
*** a/src/backend/utils/adt/int8.c
--- b/src/backend/utils/adt/int8.c
***************
*** 20,25 ****
--- 20,26 ----
#include "funcapi.h"
#include "libpq/pqformat.h"
#include "utils/int8.h"
+ #include "utils/builtins.h"
#define MAXINT8LEN 25
*************** Datum
*** 157,170 ****
int8out(PG_FUNCTION_ARGS)
{
int64 val = PG_GETARG_INT64(0);
! char *result;
! int len;
! char buf[MAXINT8LEN + 1];
!
! if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)
! elog(ERROR, "could not format int8");
! result = pstrdup(buf);
PG_RETURN_CSTRING(result);
}
--- 158,166 ----
int8out(PG_FUNCTION_ARGS)
{
int64 val = PG_GETARG_INT64(0);
! char *result = palloc(MAXINT8LEN + 1);
! pg_lltoa(val, result);
PG_RETURN_CSTRING(result);
}
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index 5f8083f..7b50549 100644
*** a/src/backend/utils/adt/numutils.c
--- b/src/backend/utils/adt/numutils.c
***************
*** 3,10 ****
* numutils.c
* utility functions for I/O of built-in numeric types.
*
- * integer: pg_atoi, pg_itoa, pg_ltoa
- *
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
--- 3,8 ----
*************** pg_atoi(char *s, int size, int c)
*** 109,135 ****
}
/*
! * pg_itoa - converts a short int to its string represention
*
! * Note:
! * previously based on ~ingres/source/gutil/atoi.c
! * now uses vendor's sprintf conversion
*/
void
pg_itoa(int16 i, char *a)
{
! sprintf(a, "%hd", (short) i);
}
/*
! * pg_ltoa - converts a long int to its string represention
*
! * Note:
! * previously based on ~ingres/source/gutil/atoi.c
! * now uses vendor's sprintf conversion
*/
void
! pg_ltoa(int32 l, char *a)
{
! sprintf(a, "%d", l);
}
--- 107,239 ----
}
/*
! * pg_ltoa - convert a signed 16bit integer to its string representation
*
! * It doesnt seem worth implementing this separately.
*/
void
pg_itoa(int16 i, char *a)
{
! pg_ltoa((int32)i, a);
}
+
/*
! * pg_ltoa: convert a signed 32bit integer to its string representation
*
! * 'buf' has to be 12 bytes long to fit the result of any 32bit integer.
! *
! * Its unfortunate to have this function twice - once for 32bit, once
! * for 64bit, but incurring the cost of 64bit computation to 32bit
! * platforms doesn't seem to be acceptable.
*/
void
! pg_ltoa(int32 value, char *buf)
{
! char *bufstart = buf;
! bool neg = false;
!
! /*
! * Avoid problems with the most negative not being representable
! * as a positive integer
! */
! if (value == INT32_MIN)
! {
! memcpy(buf, "-2147483648", 12);
! return;
! }
! else if (value < 0)
! {
! value = -value;
! neg = true;
! }
!
! /* Build the string by computing the wanted string backwards. */
! do
! {
! int32 remainder;
! int32 oldval = value;
! /*
! * division by constants can be optimized by some modern
! * compilers (including gcc). We could add the concrete,
! * optimized, calculatation here to be fast at -O0 and/or
! * other compilers... Not sure if its worth doing.
! */
! value /= 10;
! remainder = oldval - value * 10;
! *buf++ = '0' + remainder;
! }
! while (value != 0);
!
! if (neg)
! *buf++ = '-';
!
! /* have to reorder the string, but not 0 byte */
! *buf-- = 0;
!
! /* reverse string */
! while (bufstart < buf)
! {
! char swap = *bufstart;
! *bufstart++ = *buf;
! *buf-- = swap;
! }
! }
!
! /*
! * pg_lltoa: convert a signed 64bit integer to its string representation
! *
! * 'buf' has to be MAXINT8LEN + 1 bytes long to fit the result of any
! * 74bit integer.
! */
! void pg_lltoa(int64 value, char *buf)
! {
! char *bufstart = buf;
! bool neg = false;
!
! /*
! * Avoid problems with the most negative not being representable as
! * a positive integer
! */
! if (value == INT64_MIN)
! {
! memcpy(buf, "-9223372036854775808", 21);
! return;
! }
! else if (value < 0)
! {
! value = -value;
! neg = true;
! }
!
! /* Build the string by computing the wanted string backwards. */
! do
! {
! int64 remainder;
! int64 oldval = value;
! /*
! * division by constants can be optimized by some modern
! * compilers (including gcc). We could add the concrete,
! * optimized, calculatation here to be fast at -O0 and/or
! * other compilers... Not sure if its worth doing.
! */
! value /= 10;
! remainder = oldval - value * 10;
! *buf++ = '0' + remainder;
! }
! while (value != 0);
!
! if (neg)
! *buf++ = '-';
!
! /* have to reorder the string, but not 0 byte */
! *buf-- = 0;
!
! /* reverse string */
! while (bufstart < buf)
! {
! char swap = *bufstart;
! *bufstart++ = *buf;
! *buf-- = swap;
! }
}
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 020ce3c..7b1bb23 100644
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
*************** extern Datum current_schemas(PG_FUNCTION
*** 275,280 ****
--- 275,281 ----
extern int32 pg_atoi(char *s, int size, int c);
extern void pg_itoa(int16 i, char *a);
extern void pg_ltoa(int32 l, char *a);
+ extern void pg_lltoa(int64 ll, char *a);
/*
* Per-opclass comparison functions for new btrees. These are
diff --git a/src/test/regress/expected/int2.out b/src/test/regress/expected/int2.out
index 8fff981..3bb26b3 100644
*** a/src/test/regress/expected/int2.out
--- b/src/test/regress/expected/int2.out
*************** SELECT '' AS five, i.f1, i.f1 / int4 '2'
*** 242,244 ****
--- 242,257 ----
| -32767 | -16383
(5 rows)
+ -- corner cases
+ SELECT (1<<15-1)::int2::text;
+ text
+ -------
+ 16384
+ (1 row)
+
+ SELECT (-1<<15)::int2::text;
+ text
+ --------
+ -32768
+ (1 row)
+
diff --git a/src/test/regress/expected/int4.out b/src/test/regress/expected/int4.out
index 3fadb7b..42095c7 100644
*** a/src/test/regress/expected/int4.out
--- b/src/test/regress/expected/int4.out
*************** SELECT (2 + 2) / 2 AS two;
*** 329,331 ****
--- 329,344 ----
2
(1 row)
+ -- corner cases
+ SELECT (1<<31-1)::int4::text;
+ text
+ ------------
+ 1073741824
+ (1 row)
+
+ SELECT (1<<31)::int4::text;
+ text
+ -------------
+ -2147483648
+ (1 row)
+
diff --git a/src/test/regress/expected/int8.out b/src/test/regress/expected/int8.out
index c8e2dad..e156067 100644
*** a/src/test/regress/expected/int8.out
--- b/src/test/regress/expected/int8.out
*************** SELECT * FROM generate_series('+45678901
*** 802,804 ****
--- 802,817 ----
4567890123456799
(6 rows)
+ -- corner cases
+ SELECT (1<<63-1)::int8::text;
+ text
+ ------------
+ 1073741824
+ (1 row)
+
+ SELECT (1<<63)::int8::text;
+ text
+ -------------
+ -2147483648
+ (1 row)
+
diff --git a/src/test/regress/sql/int2.sql b/src/test/regress/sql/int2.sql
index 20d31b6..8bffe53 100644
*** a/src/test/regress/sql/int2.sql
--- b/src/test/regress/sql/int2.sql
*************** SELECT '' AS five, i.f1, i.f1 - int4 '2'
*** 83,85 ****
--- 83,89 ----
SELECT '' AS five, i.f1, i.f1 / int2 '2' AS x FROM INT2_TBL i;
SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
+
+ -- corner cases
+ SELECT (1<<15-1)::int2::text;
+ SELECT (-1<<15)::int2::text;
diff --git a/src/test/regress/sql/int4.sql b/src/test/regress/sql/int4.sql
index 5e84f64..39bfec1 100644
*** a/src/test/regress/sql/int4.sql
--- b/src/test/regress/sql/int4.sql
*************** SELECT 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1
*** 123,125 ****
--- 123,129 ----
SELECT 2 + 2 / 2 AS three;
SELECT (2 + 2) / 2 AS two;
+
+ -- corner cases
+ SELECT (1<<31-1)::int4::text;
+ SELECT (1<<31)::int4::text;
diff --git a/src/test/regress/sql/int8.sql b/src/test/regress/sql/int8.sql
index 648563c..7fff03c 100644
*** a/src/test/regress/sql/int8.sql
--- b/src/test/regress/sql/int8.sql
*************** SELECT q1, q1 << 2 AS "shl", q1 >> 3 AS
*** 190,192 ****
--- 190,196 ----
SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::int8);
SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::int8, 0);
SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::int8, 2);
+
+ -- corner cases
+ SELECT (1<<63-1)::int8::text;
+ SELECT (1<<63)::int8::text;
--
1.7.3.rc1.5.g73aa2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
