Re: Missing free_var() at end of accum_sum_final()?

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