Thread: Re: [COMMITTERS] pgsql: Invent a memory context reset/delete callback mechanism.
Re: [COMMITTERS] pgsql: Invent a memory context reset/delete callback mechanism.
From
Jeff Davis
Date:
On Fri, 2015-02-27 at 22:17 +0000, Tom Lane wrote: > In passing, per discussion, rearrange some boolean fields in struct > MemoryContextData so as to avoid wasted padding space. For safety, > this requires making allowInCritSection's existence unconditional; > but I think that's a better approach than what was there anyway. I notice that this uses the bytes in MemoryContextData that I was hoping to use for the memory accounting patch (for memory-bounded HashAgg). Leaving no space for even a 32-bit value (without AllocSetContext growing beyond the magical 192-byte number Andres mentioned) removes most of the options for memory accounting. The only things I can think of are: 1. Move a few less-frequently-accessed fields out to an auxiliary structure (Tomas proposed something similar); or 2. Walk the memory contexts, but use some kind of estimation/extrapolation so that it doesn't need to be done for every input tuple of HashAgg. Thoughts? Regards, Jeff Davis
Re: Re: [COMMITTERS] pgsql: Invent a memory context reset/delete callback mechanism.
From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes: > On Fri, 2015-02-27 at 22:17 +0000, Tom Lane wrote: >> In passing, per discussion, rearrange some boolean fields in struct >> MemoryContextData so as to avoid wasted padding space. For safety, >> this requires making allowInCritSection's existence unconditional; >> but I think that's a better approach than what was there anyway. > I notice that this uses the bytes in MemoryContextData that I was hoping > to use for the memory accounting patch (for memory-bounded HashAgg). Meh. I thought Andres' complaint about that was unfounded anyway ;-). But I don't really see why a memory accounting patch should be expected to have zero footprint. regards, tom lane
Re: Re: [COMMITTERS] pgsql: Invent a memory context reset/delete callback mechanism.
From
Stephen Frost
Date:
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Jeff Davis <pgsql@j-davis.com> writes: > > On Fri, 2015-02-27 at 22:17 +0000, Tom Lane wrote: > >> In passing, per discussion, rearrange some boolean fields in struct > >> MemoryContextData so as to avoid wasted padding space. For safety, > >> this requires making allowInCritSection's existence unconditional; > >> but I think that's a better approach than what was there anyway. > > > I notice that this uses the bytes in MemoryContextData that I was hoping > > to use for the memory accounting patch (for memory-bounded HashAgg). > > Meh. I thought Andres' complaint about that was unfounded anyway ;-). > But I don't really see why a memory accounting patch should be expected to > have zero footprint. For my 2c, I agree that it's a bit ugly to waste space due to padding, but I'm all about improving the memory accounting and would feel that's well worth having a slightly larger MemoryContextData. In other words, I agree with Tom that it doesn't need to have a zero footprint, but disagree about wasting space due to padding. :D Thanks! Stephen