On Fri, Mar 3, 2023, at 16:11, Dean Rasheed wrote:
> So IMO the results just don't justify such an extensive set of
> changes, and I think we should abandon this fixed buffer approach.
I agree. I was hoping it would be possible to reduce the invasiveness,
but I think it's difficult and probably not worth it.
> Having said that, I think the results from the COPY test are worth
> looking at more closely. Your results seem to suggest around a 5%
> improvement. On my machine it was only around 3%, but that still might
> be worth having, if it didn't involve such invasive changes throughout
> the rest of the code.
>
> As an experiment, I took another look at my earlier patch, making
> make_result() construct the result using the same allocated memory as
> the variable's digit buffer (if allocated). That eliminates around a
> third of all free_var() calls from numeric.c, and for most functions,
> it saves both a palloc() and a pfree(). In the case of numeric_in(), I
> realised that it's possible to go further, by reusing the decimal
> digits buffer for the NumericVar's digits, and then later for the
> final Numeric result. Also, by carefully aligning things, it's
> possible to arrange it so that the final make_result() doesn't need to
> copy/move the digits at all. With that I get something closer to a 15%
> improvement in the COPY test, which is definitely worth having.
Nice! Patch LGTM.
> I didn't do all the tests that you did though, so it would be
> interesting to see how it fares in those.
SELECT count(*) FROM generate_series(1::numeric, 10000000::numeric);
Time: 1196.801 ms (00:01.197) -- HEAD
Time: 1278.376 ms (00:01.278) -- make-result-using-vars-buf-v2.patch
TRUNCATE foo; COPY foo FROM '/tmp/random-numerics.csv';
Time: 8450.551 ms (00:08.451) -- HEAD
Time: 7176.838 ms (00:07.177) -- make-result-using-vars-buf-v2.patch
SELECT SUM(n1+n2+n3+n4) FROM t;
Time: 643.961 ms -- HEAD
Time: 620.303 ms -- make-result-using-vars-buf-v2.patch
SELECT max(a + b + '17'::numeric + c)
FROM
(SELECT generate_series(1::numeric, 1000::numeric)) aa(a),
(SELECT generate_series(1::numeric, 100::numeric)) bb(b),
(SELECT generate_series(1::numeric, 10::numeric)) cc(c);
Time: 141.070 ms -- HEAD
Time: 139.562 ms -- make-result-using-vars-buf-v2.patch
SELECT SUM(amount*1.25 + 0.5) FROM ledger;
Time: 933.461 ms -- HEAD
Time: 862.619 ms -- make-result-using-vars-buf-v2.patch
Looks like a win in all cases except the first one.
Great work!
/Joel
