public inbox for [email protected]
help / color / mirror / Atom feedFrom: Joel Jacobson <[email protected]>
To: [email protected]
Subject: Re: Missing free_var() at end of accum_sum_final()?
Date: Sun, 19 Feb 2023 23:16:43 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
Hi again,
Ignore previous patch, new correct version attached, that also keeps track of if the buf-field is in use or not.
Seems like your idea gives a signifiant speed-up!
On my machine, numeric_in() is 22% faster!
I ran a benchmark with 100 tests measuring execution-time of numeric_in() with two significant digits time precision.
Benchmark:
CREATE EXTENSION pit;
SELECT count(pit.async('numeric_in','{1234,0,-1}',2)) FROM generate_series(1,100);
CALL pit.work(true);
SELECT
format('%s(%s)',pit.test_params.function_name, array_to_string(pit.test_params.input_values,',')),
pit.pretty_time(pit.tests.final_result),
count(*)
FROM pit.tests
JOIN pit.test_params USING (id)
GROUP BY 1,2
ORDER BY 1,2;
HEAD:
format | pretty_time | count
-----------------------+-------------+-------
numeric_in(1234,0,-1) | 31 ns | 34
numeric_in(1234,0,-1) | 32 ns | 66
(2 rows)
0002-fixed-buf.patch:
format | pretty_time | count
-----------------------+-------------+-------
numeric_in(1234,0,-1) | 24 ns | 4
numeric_in(1234,0,-1) | 25 ns | 71
numeric_in(1234,0,-1) | 26 ns | 25
(3 rows)
The benchmark results was produced with: https://github.com/joelonsql/pg-timeit
Now, the question is how big of a fixed_buf we want. I will run some more benchmarks.
/Joel
On Sun, Feb 19, 2023, at 20:54, Joel Jacobson wrote:
> On Fri, Feb 17, 2023, at 21:26, Andres Freund wrote:
>> Removing the free_var()s from numeric_add_opt_error() speeds it up from ~361ms
>> to ~338ms.
>
> I notice numeric_add_opt_error() is extern and declared in numeric.h,
> called from e.g. timestamp.c and jsonpath_exec.c. Is that a problem,
> i.e. is there a risk it could be used in a for loop by some code
> outside Numeric?
>
>> This code really needs some memory management overhead reduction love. Many
>> allocation could be avoided by having a small on-stack "buffer" that's used
>> unless the numeric is large.
>
> Nice idea!
> Could something like the attached patch work?
>
> /Joel
> Attachments:
> * 0001-fixed-buf.patch
--
Kind regards,
Joel
Attachments:
[application/octet-stream] 0002-fixed-buf.patch (3.7K, ../[email protected]/2-0002-fixed-buf.patch)
download | inline diff:
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index a83feea396..485b072394 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -301,6 +301,7 @@ struct NumericData
* This is feasible because the digit buffer is separate from the variable.
* ----------
*/
+#define FIXED_BUF_LEN 4
typedef struct NumericVar
{
int ndigits; /* # of digits in digits[] - can be 0! */
@@ -309,6 +310,8 @@ typedef struct NumericVar
int dscale; /* display scale */
NumericDigit *buf; /* start of palloc'd space for digits[] */
NumericDigit *digits; /* base-NBASE digits */
+ bool is_allocated; /* is buf palloc'd? */
+ NumericDigit fixed_buf[FIXED_BUF_LEN];
} NumericVar;
@@ -414,18 +417,18 @@ typedef struct NumericSumAccum
*/
static const NumericDigit const_zero_data[1] = {0};
static const NumericVar const_zero =
-{0, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_zero_data};
+{0, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_zero_data, false, {0}};
static const NumericDigit const_one_data[1] = {1};
static const NumericVar const_one =
-{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_one_data};
+{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_one_data, false, {0}};
static const NumericVar const_minus_one =
-{1, 0, NUMERIC_NEG, 0, NULL, (NumericDigit *) const_one_data};
+{1, 0, NUMERIC_NEG, 0, NULL, (NumericDigit *) const_one_data, false, {0}};
static const NumericDigit const_two_data[1] = {2};
static const NumericVar const_two =
-{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_two_data};
+{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_two_data, false, {0}};
#if DEC_DIGITS == 4
static const NumericDigit const_zero_point_nine_data[1] = {9000};
@@ -435,7 +438,7 @@ static const NumericDigit const_zero_point_nine_data[1] = {90};
static const NumericDigit const_zero_point_nine_data[1] = {9};
#endif
static const NumericVar const_zero_point_nine =
-{1, -1, NUMERIC_POS, 1, NULL, (NumericDigit *) const_zero_point_nine_data};
+{1, -1, NUMERIC_POS, 1, NULL, (NumericDigit *) const_zero_point_nine_data, false, {0}};
#if DEC_DIGITS == 4
static const NumericDigit const_one_point_one_data[2] = {1, 1000};
@@ -445,16 +448,16 @@ static const NumericDigit const_one_point_one_data[2] = {1, 10};
static const NumericDigit const_one_point_one_data[2] = {1, 1};
#endif
static const NumericVar const_one_point_one =
-{2, 0, NUMERIC_POS, 1, NULL, (NumericDigit *) const_one_point_one_data};
+{2, 0, NUMERIC_POS, 1, NULL, (NumericDigit *) const_one_point_one_data, false, {0}};
static const NumericVar const_nan =
-{0, 0, NUMERIC_NAN, 0, NULL, NULL};
+{0, 0, NUMERIC_NAN, 0, NULL, NULL, false, {0}};
static const NumericVar const_pinf =
-{0, 0, NUMERIC_PINF, 0, NULL, NULL};
+{0, 0, NUMERIC_PINF, 0, NULL, NULL, false, {0}};
static const NumericVar const_ninf =
-{0, 0, NUMERIC_NINF, 0, NULL, NULL};
+{0, 0, NUMERIC_NINF, 0, NULL, NULL, false, {0}};
#if DEC_DIGITS == 4
static const int round_powers[4] = {0, 1000, 100, 10};
@@ -6871,10 +6874,22 @@ dump_var(const char *str, NumericVar *var)
static void
alloc_var(NumericVar *var, int ndigits)
{
- digitbuf_free(var->buf);
- var->buf = digitbuf_alloc(ndigits + 1);
- var->buf[0] = 0; /* spare digit for rounding */
- var->digits = var->buf + 1;
+ if (var->is_allocated)
+ digitbuf_free(var->buf);
+ if (ndigits < FIXED_BUF_LEN - 1)
+ {
+ var->is_allocated = false;
+ var->fixed_buf[0] = 0; /* spare digit for rounding */
+ var->digits = var->fixed_buf + 1;
+
+ }
+ else
+ {
+ var->is_allocated = true;
+ var->buf = digitbuf_alloc(ndigits + 1);
+ var->buf[0] = 0; /* spare digit for rounding */
+ var->digits = var->buf + 1;
+ }
var->ndigits = ndigits;
}
view thread (8+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Missing free_var() at end of accum_sum_final()?
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox