From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy
Date: Sun, 12 Aug 2018 11:25:39 +0100
Message-ID: <87bma77v67.fsf@news-spur.riddles.org.uk> (raw)
In-Reply-To: <CAFj8pRDWORKuuLEvFN-tMp06nuD5g_+yPAWWVW7umanoriEwwg@mail.gmail.com>
References: <153394403528.10284.7530399040974170549@wrigleys.postgresql.org>
<87va8h7nyl.fsf@news-spur.riddles.org.uk>
<87r2j57hdo.fsf@news-spur.riddles.org.uk>
<CAFj8pRAm9cRuTf5STnX=0fGLpxhu7CR6GAj8LGq+iuw+NKXfPg@mail.gmail.com>
<CAFj8pRBMpOn7X-N2PZKFtMK2UNjTahoArGmtMo1C7Az0x9Rzeg@mail.gmail.com>
<87k1ow6o7i.fsf@news-spur.riddles.org.uk>
<CAFj8pRD7bkGhn47wTw-CPhAQgH8OikezB=JrzVRsP_RAWH-scA@mail.gmail.com>
<87ftzk6jw2.fsf@news-spur.riddles.org.uk>
<CAFj8pRDWORKuuLEvFN-tMp06nuD5g_+yPAWWVW7umanoriEwwg@mail.gmail.com>
>>>>> "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_.
--
Andrew (irc:RhodiumToad)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: andrew@tao11.riddles.org.uk, pavel.stehule@gmail.com, alvherre@2ndquadrant.com
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy
In-Reply-To: <87bma77v67.fsf@news-spur.riddles.org.uk>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox