On 05.03.23 09:53, Joel Jacobson wrote:
On Fri, Mar 3, 2023, at 16:11, Dean Rasheed wrote:
Attachments:
* make-result-using-vars-buf-v2.patch
One suggestion: maybe add a comment explaining why the allocated buffer
which size is based on strlen(cp) for the decimal digit values,
is guaranteed
On Fri, Mar 3, 2023, at 16:11, Dean Rasheed wrote:
> Attachments:
> * make-result-using-vars-buf-v2.patch
One suggestion: maybe add a comment explaining why the allocated buffer
which size is based on strlen(cp) for the decimal digit values,
is guaranteed to be large enough also for the result's
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
> On 20.02.23 23:16, Joel Jacobson wrote:
> > In the new attached patch, Andres fixed buffer idea has been implemented
> > throughout the entire numeric.c code base.
>
I have been going over this patch, and IMO it's far too invasive for
the fairly modest performance gains (and performance
On 20.02.23 23:16, Joel Jacobson wrote:
In the new attached patch, Andres fixed buffer idea has been implemented
throughout the entire numeric.c code base.
I think the definition of the "preinitialized constants" could be
adapted to this as well. For example, instead of
static const
Hi,
My apologies, it seems my email didn't reach the list, probably due to the
benchmark plot images being to large. Here is the email again, but with
URLs to the images instead, and benchmark updated with results for the
0005-fixed-buf.patch.
--
On Mon, Feb 20, 2023, at 12:32, Dean Rasheed
Hi,
I found another small but significant improvement of the previous patch:
else if (ndigits < var->buf_len)
{
-memset(var->buf, 0, var->buf_len);
+var->buf[0] = 0;
var->digits = var->buf + 1;
var->ndigits = ndigits;
}
We don't need to set all buf elements to zero, only the
On Mon, 20 Feb 2023 at 08:03, Joel Jacobson wrote:
>
> On Mon, Feb 20, 2023, at 08:38, Michael Paquier wrote:
> > Perhaps you should register this patch to the commit of March? Here
> > it is:
> > https://commitfest.postgresql.org/42/
>
> Thanks, done.
>
I have been testing this a bit, and I
On Mon, Feb 20, 2023, at 08:38, Michael Paquier wrote:
> Perhaps you should register this patch to the commit of March? Here
> it is:
> https://commitfest.postgresql.org/42/
Thanks, done.
/Joel
On Sun, Feb 19, 2023 at 11:55:38PM +0100, Joel Jacobson wrote:
> To fix, a init_var() in alloc_var() is needed when we will use the
> fixed_buf instead of allocating,
> since alloc_var() could be called in a loop for existing values,
> like in sqrt_var() for instance.
>
> Attached new version
On Sun, Feb 19, 2023, at 23:16, Joel Jacobson wrote:
> Hi again,
>
> Ignore previous patch, new correct version attached, that also keeps
> track of if the buf-field is in use or not.
Ops! One more thinko, detected when trying fixed_buf[8], which caused a seg
fault.
To fix, a init_var() in
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
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
Hi,
On 2023-02-17 11:48:14 +0900, Michael Paquier wrote:
> On Thu, Feb 16, 2023 at 01:35:54PM -0800, Andres Freund wrote:
> > But why do we need it? Most SQL callable functions don't need to be careful
> > about not leaking O(1) memory, the exception being functions backing btree
> > opclasses.
>
On Thu, Feb 16, 2023 at 01:35:54PM -0800, Andres Freund wrote:
> But why do we need it? Most SQL callable functions don't need to be careful
> about not leaking O(1) memory, the exception being functions backing btree
> opclasses.
>
> In fact, the detailed memory management often is *more*
Hi,
On 2023-02-16 15:26:26 +0900, Michael Paquier wrote:
> On Thu, Feb 16, 2023 at 06:59:13AM +0100, Joel Jacobson wrote:
> > I noticed the NumericVar's pos_var and neg_var are not free_var()'d
> > at the end of accum_sum_final().
> >
> > The potential memory leak seems small, since the function
On Thu, Feb 16, 2023 at 08:08:37AM +0100, Joel Jacobson wrote:
> I added the free_var() calls after strip_var() to match the similar
> existing code at the end of sqrt_var().
> Not that it matters, but thought it looked nicer to keep them in the
> same order.
WFM. Thanks!
--
Michael
On Thu, Feb 16, 2023, at 07:26, Michael Paquier wrote:
> Indeed, it is true that any code path of numeric.c that relies on a
> NumericVar with an allocation done in its buffer is careful enough to
> free it, except for generate_series's SRF where one step of the
> computation is done. I don't see
On Thu, Feb 16, 2023 at 06:59:13AM +0100, Joel Jacobson wrote:
> I noticed the NumericVar's pos_var and neg_var are not free_var()'d
> at the end of accum_sum_final().
>
> The potential memory leak seems small, since the function is called
> only once per sum() per worker (and from a few more
Hi,
I noticed the NumericVar's pos_var and neg_var are not free_var()'d at the end
of accum_sum_final().
The potential memory leak seems small, since the function is called only once
per sum() per worker (and from a few more places), but maybe they should be
free'd anyways for correctness?
20 matches
Mail list logo
