Re: Make MemoryContextMemAllocated() more precise

2020-07-01 Thread Daniel Gustafsson
> On 8 Apr 2020, at 02:21, Jeff Davis wrote: > The attached patch is narrow and solves the problem for HashAgg nicely > without interfering with anything else, so I plan to commit it soon for > v13. If I read this thread correctly, there is nothing outstanding here for 14 from this patch? I've

Re: Make MemoryContextMemAllocated() more precise

2020-04-07 Thread Jeff Davis
On Mon, 2020-03-16 at 11:45 -0700, Jeff Davis wrote: > AllocSet allocates memory for itself in blocks, which double in size > up > to maxBlockSize. So, the current block (the last one malloc'd) may > represent half of the total memory allocated for the context itself. Narrower approach that

Re: Make MemoryContextMemAllocated() more precise

2020-04-07 Thread David Rowley
On Tue, 7 Apr 2020 at 14:26, Jeff Davis wrote: > > On Mon, 2020-04-06 at 23:39 +1200, David Rowley wrote: > > 4. Do you think it would be better to have two separate functions for > > MemoryContextCount(), a recursive version and a non-recursive > > version. > > I could, but right now the only

Re: Make MemoryContextMemAllocated() more precise

2020-04-06 Thread Jeff Davis
On Sun, 2020-04-05 at 16:48 -0700, Andres Freund wrote: > > I still think we should do something for v13, such as the > > originally- > > proposed patch[1]. It's not critical, but it simply reports a > > better > > number for memory consumption. Currently, the memory usage appears > > to > > jump,

Re: Make MemoryContextMemAllocated() more precise

2020-04-06 Thread Jeff Davis
On Mon, 2020-04-06 at 23:39 +1200, David Rowley wrote: > 1. The comment mentions "passthru", but you've removed that > parameter. Fixed, thank you. > 2. I don't think MemoryContextCount is the best name for this > function. When I saw: I've gone back and forth on naming a bit. The right name,

Re: Make MemoryContextMemAllocated() more precise

2020-04-06 Thread David Rowley
On Sat, 28 Mar 2020 at 13:21, Jeff Davis wrote: > Attached refactoring patch. There's enough in here that warrants > discussion that I don't think this makes sense for v13 and I'm adding > it to the July commitfest. I had a read over this too. I noted down the following during my pass of it. 1.

Re: Make MemoryContextMemAllocated() more precise

2020-04-05 Thread Andres Freund
Hi, On 2020-03-27 17:21:10 -0700, Jeff Davis wrote: > Attached refactoring patch. There's enough in here that warrants > discussion that I don't think this makes sense for v13 and I'm adding > it to the July commitfest. IDK, adding a commit to v13 that we know we should do architecturally

Re: Make MemoryContextMemAllocated() more precise

2020-03-27 Thread Jeff Davis
On Wed, 2020-03-18 at 15:41 -0700, Jeff Davis wrote: > In an off-list discussion, Andres suggested that MemoryContextStats > could be refactored to achieve this purpose, perhaps with flags to > avoid walking through the blocks and freelists when those are not > needed. Attached refactoring patch.

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Jeff Davis
On Thu, 2020-03-19 at 16:04 -0400, Robert Haas wrote: > Other people may have different concerns, but that was the only thing > that was bothering me. OK, thank you for raising it. Perhaps we can re-fix the issue for HashAgg if necessary, or I can tweak some accounting things within HashAgg

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Jeff Davis
On Thu, 2020-03-19 at 15:33 -0400, Robert Haas wrote: > On Thu, Mar 19, 2020 at 3:27 PM Jeff Davis wrote: > > I think omitting the tail of the current block is an unqualified > > improvement for the purpose of obeying work_mem, regardless of the > > OS. > > The block sizes keep doubling up to

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Robert Haas
On Thu, Mar 19, 2020 at 3:44 PM Jeff Davis wrote: > On Thu, 2020-03-19 at 15:26 -0400, Robert Haas wrote: > > Well, the issue is, if I understand correctly, that this means that > > MemoryContextStats() might now report a smaller amount of memory than > > what we actually allocated from the

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Jeff Davis
On Mon, 2020-03-16 at 11:45 -0700, Jeff Davis wrote: > Although that's technically correct, the purpose of > MemoryContextMemAllocated() is to give a "real" usage number so we > know > when we're out of work_mem and need to spill (in particular, the > disk- > based HashAgg work, but ideally other

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Jeff Davis
On Thu, 2020-03-19 at 15:26 -0400, Robert Haas wrote: > Well, the issue is, if I understand correctly, that this means that > MemoryContextStats() might now report a smaller amount of memory than > what we actually allocated from the operating system. That seems like > it might lead someone trying

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Tomas Vondra
On Thu, Mar 19, 2020 at 12:25:16PM -0700, Jeff Davis wrote: On Thu, 2020-03-19 at 19:11 +0100, Tomas Vondra wrote: AFAICS the 2x allocation is the worst case, because it only happens right after allocating a new block (of twice the size), when the "utilization" drops from 100% to 50%. But in

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Robert Haas
On Thu, Mar 19, 2020 at 3:27 PM Jeff Davis wrote: > I think omitting the tail of the current block is an unqualified > improvement for the purpose of obeying work_mem, regardless of the OS. > The block sizes keep doubling up to 8MB, and it doesn't make a lot of > sense to count that last large

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Jeff Davis
On Thu, 2020-03-19 at 11:44 -0400, Robert Haas wrote: > Procedurally, I think that it is highly inappropriate to submit a > patch two weeks after the start of the final CommitFest and then > commit it just over 48 hours later without a single endorsement of > the > change from anyone. Reverted.

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Robert Haas
On Thu, Mar 19, 2020 at 2:11 PM Tomas Vondra wrote: > My understanding is that this is really just an accounting issue, where > allocating a block would get us over the limit, which I suppose might be > an issue with low work_mem values. Well, the issue is, if I understand correctly, that this

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Jeff Davis
On Thu, 2020-03-19 at 19:11 +0100, Tomas Vondra wrote: > AFAICS the 2x allocation is the worst case, because it only happens > right after allocating a new block (of twice the size), when the > "utilization" drops from 100% to 50%. But in practice the utilization > will be somewhere in between,

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Tomas Vondra
On Thu, Mar 19, 2020 at 11:44:05AM -0400, Robert Haas wrote: On Mon, Mar 16, 2020 at 2:45 PM Jeff Davis wrote: Attached is a patch that makes mem_allocated a method (rather than a field) of MemoryContext, and allows each memory context type to track the memory its own way. They all do the same

Re: Make MemoryContextMemAllocated() more precise

2020-03-19 Thread Robert Haas
On Mon, Mar 16, 2020 at 2:45 PM Jeff Davis wrote: > Attached is a patch that makes mem_allocated a method (rather than a > field) of MemoryContext, and allows each memory context type to track > the memory its own way. They all do the same thing as before > (increment/decrement a field), but

Re: Make MemoryContextMemAllocated() more precise

2020-03-18 Thread Jeff Davis
On Mon, 2020-03-16 at 11:45 -0700, Jeff Davis wrote: > Attached is a patch that makes mem_allocated a method (rather than a > field) of MemoryContext, and allows each memory context type to track > the memory its own way. They all do the same thing as before > (increment/decrement a field), but

Make MemoryContextMemAllocated() more precise

2020-03-16 Thread Jeff Davis
AllocSet allocates memory for itself in blocks, which double in size up to maxBlockSize. So, the current block (the last one malloc'd) may represent half of the total memory allocated for the context itself. The free space at the end of that block hasn't been touched at all, and doesn't