Re: BUG #15321: XMLTABLE leaks memory like crazy

2018-08-12 13:18 GMT+02:00 Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>:

> >>>>> "Pavel" == Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>
> >> 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_.
>
> Pavel> ok, now I am maybe understand to your motivation.
>
> Pavel> Usually, loading row should be memory cheap operation, but sure
> Pavel> some bytes it can take.
>
> Yes, it'll usually be small, but you shouldn't assume that (and some
> type input functions may use more memory than you think, since doing a
> lot of retail pfree() calls can really slow things down).
>
> Pavel> Then I don't like too much using ecxt_per_tuple_memory for this.
>
> It's there, it has the right lifetime, allocating another one just for
> this is a waste. Furthermore, this is the same pattern that FunctionScan
> uses, so it's more consistent.
>
> Pavel> Or do better comments about using this memory context for very
> Pavel> short life task, please.
>
> What specifically do you think needs explaining?
>

I don't feel well, when context named GetValue has longer life than
ecxt_tuple_memory context. I understand so it is not important in SRF
function, but it looks strange for me.

It is my subjective option, and I have not any strong arguments for it. If
commiter think so it is ok, then I can live with it :)

> Attached patch is the same logic as before but with more comments.
>

ok

Regards

Pavel

> --
> Andrew (irc:RhodiumToad)
>
>