Re: Reduce palloc's in numeric operations.

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: hlinnakangas(at)vmware(dot)com
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Reduce palloc's in numeric operations.
Date: 2012-11-12 09:45:15
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Thanks for comments,

> Have to be careful to really not modify the
> operands. numeric_out() and numeric_out_sci() are wrong; they
> call get_str_from_var(), which modifies the argument. Same with
> numeric_expr(): it passes the argument to
> numericvar_to_double_no_overflow(), which passes it to
> get_str_from_var(). numericvar_to_int8() also modifies its
> argument, so all the functions that use that, directly or
> indirectly, must make a copy.

mmm. My carefulness was a bit short to pick up them...

I overlooked that get_str_from_var() and numeric_to_int8() calls
round_var() which rewrites the operand. I reverted numeric_out()
and numeric_int8(), numeric_int2().

Altough, I couldn't find in get_str_from_var_sci() where the
operand would be modified.

The lines where var showing in get_str_from_var_sci() is listed

| 2:get_str_from_var_sci(NumericVar *var, int rscale)
| 21: if (var->ndigits > 0)
| 23: exponent = (var->weight + 1) * DEC_DIGITS;
| 29: exponent -= DEC_DIGITS - (int) log10(var->digits[0]);
| 59: div_var(var, &denominator, &significand, rscale, true);

The only suspect is div_var at this level, and do the same thing
for var1 in div_var.

| 2:div_var(NumericVar *var1, NumericVar *var2, NumericVar *result,
| 20: int var1ndigits = var1->ndigits;
| 35: if (var1ndigits == 0)
| 47: if (var1->sign == var2->sign)
| 51: res_weight = var1->weight - var2->weight;
| 68: div_ndigits = Max(div_ndigits, var1ndigits);
| 83: memcpy(dividend + 1, var1->digits, var1ndigits * sizeof(NumericDigit));
| 132: for (i = var1ndigits; i >= 0; i--)

No line seems to modify var1 as far as I see so I've left
numeric_out_sci() modified in this patch.

Well, I found some other bugs in numeric_stddev_internal.
vN was errorniously freed and vsumX2 in is used as work.
They are fixed in this new patch.

> Perhaps get_str_from_var(), and the other functions that
> currently scribble on the arguments, should be modified to not
> do so. They could easily make a copy of the argument within the
> function. Then the callers could safely use
> set_var_from_num_nocopy(). The performance would be the same,
> you would have the same number of pallocs, but you would get
> rid of the surprising argument-modifying behavior of those
> functions.

I agree with that. const qualifiers on parameters would rule this
mechanically. I try this for the next version of this patch.

> SELECT SUM(col) FROM numtest;
> The execution time of that query fell from about 5300 ms to 4300 ms, ie. about 20%.

Wow, it seems more promising than I expected. Thanks.



日本電信電話株式会社 NTTオープンソフトウェアセンタ
Phone: 03-5860-5115 / Fax: 03-5463-5490

Attachment Content-Type Size
fasternumeric_20121112_v2.patch text/x-patch 8.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2012-11-12 10:07:07 Re: Identity projection
Previous Message Craig Ringer 2012-11-12 09:44:27 Re: Enabling Checksums