Re: BUG #15321: XMLTABLE leaks memory like crazy - Mailing list pgsql-bugs

From Pavel Stehule
Subject Re: BUG #15321: XMLTABLE leaks memory like crazy
Date
Msg-id CAFj8pRDr74=wxRD68EGMS6Z90rTn4JOp6g9s_3BE37gtkJAX6A@mail.gmail.com
Whole thread Raw
In response to Re: BUG #15321: XMLTABLE leaks memory like crazy  (Andrew Gierth <andrew@tao11.riddles.org.uk>)
Responses Re: BUG #15321: XMLTABLE leaks memory like crazy
List pgsql-bugs


2018-08-12 12:25 GMT+02:00 Andrew Gierth <andrew@tao11.riddles.org.uk>:
>>>>> "Pavel" == Pavel Stehule <pavel.stehule@gmail.com> writes:

 >> No, tuplestore_putvalues is only responsible for the memory it
 >> allocates itself, which belongs to the tuplestore, it has nothing to
 >> do with the memory allocated by its caller - and XmlTableGetValue
 >> does quite a few allocations.

 Pavel> Maybe I used wrong words. Sorry, my English lang is not good.

 Pavel> In master tfuncLoadRows switch to tstate->perValueCxt. You
 Pavel> change it by switch econtext->ecxt_per_tuple_memory what is
 Pavel> wrong I thing.

 Pavel> If we don't change memory context, we will stay inside
 Pavel> perValueCxt. And this context will be cleaned outside.

Yes, but it won't be cleaned up until _all_ the rows from a single call
have been generated, which means that the allocations from GetValue have
been (uselessly) accumulating during this time, wasting memory.

We do need a context that is reset for every output row, as perValueCxt
used to be. I don't see why this is even in question.

 >> Before, you were using perValueCxt and resetting it once per GetValue
 >> call. Since my patch takes perValueCxt to use for another purpose
 >> instead, it needs to be replaced, and econtext->ecxt_per_tuple_memory
 >> is suitable for the job (and consistent with functionscan).

 Pavel> I think so using cxt_per_tuple_memory is not necessary - and my
 Pavel> patch is working too.

Working, just using more memory than it needs to.

 Pavel> Just I removed MemoryContextReset(tstate->perValueCxt) after
 Pavel> tuplestore_putvalues. It is possible, because
 Pavel> tstate->perValueCxt is cleaned immediately in caller
 Pavel> tfuncFetchRows function.

But that's not "immediately" because tfuncLoadRows is looping over the
FetchRow call, and calling GetValue for each column in that row, and in
your version it is _not cleaning up memory in that loop_.

ok, now I am maybe understand to your motivation.

Usually, loading row should be memory cheap operation, but sure some bytes it can take.

Then I don't like too much using ecxt_per_tuple_memory for this. Maybe better to create own short life context for purpose. Or do better comments about using this memory context for very short life task, please.

Regards

Pavel


 

--
Andrew (irc:RhodiumToad)

pgsql-bugs by date:

Previous
From: Andrew Gierth
Date:
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy
Next
From: Andrew Gierth
Date:
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy