Re: Missing free_var() at end of accum_sum_final()?
От | Andres Freund |
---|---|
Тема | Re: Missing free_var() at end of accum_sum_final()? |
Дата | |
Msg-id | 20230217202626.ihd55rgxgkr2uqim@awork3.anarazel.de обсуждение исходный текст |
Ответ на | Re: Missing free_var() at end of accum_sum_final()? (Michael Paquier <michael@paquier.xyz>) |
Ответы |
Re: Missing free_var() at end of accum_sum_final()?
("Joel Jacobson" <joel@compiler.org>)
|
Список | pgsql-hackers |
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. > > > > In fact, the detailed memory management often is *more* expensive than just > > relying on the calling memory context being reset. > > > > Of course, numeric.c doesn't really seem to have gotten that message, so > > there's a consistency argument here. > > I don't know which final result is better. The arguments go two ways: > 1) Should numeric.c be simplified so as its memory structure with extra > pfree()s, making it more consistent with more global assumptions than > just this file? This has the disadvantage of creating more noise in > backpatching, while increasing the risk of leaks if some of the > removed parts are allocated in a tight loop within the same context. > This makes memory management less complicated. That's how I am > understanding your point. It's not just simplification, it's just faster to free via context reset. I whipped up a random query exercising numeric math a bunch: 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); Removing the free_var()s from numeric_add_opt_error() speeds it up from ~361ms to ~338ms. This code really needs some memory management overhead reduction love. Many allocation could be avoided by having a small on-stack "buffer" that's used unless the numeric is large. > 2) Should the style within numeric.c be more consistent? This is how > I am understanding this proposal. As you quote, this makes memory > management more complicated (not convinced about that for the internal > of numerics?), while making the file more consistent. > At the end, perhaps that's not worth bothering, but 2) prevails when > it comes to the rule of making some code consistent with its > surroundings. 1) has more risks seeing how old this code is. I'm a bit wary that this will trigger a stream of patches to pointlessly free things, causing churn and slowdowns. I suspect there's other places in numeric.c where we don't free, and there certainly are a crapton in other functions. Greetings, Andres Freund
В списке pgsql-hackers по дате отправления: