Re: Memory Accounting v11
| От | David Rowley |
|---|---|
| Тема | Re: Memory Accounting v11 |
| Дата | |
| Msg-id | CAKJS1f8yvvvj-sVDv_bcxkzcZKq0ZOTVhX0dHfnYDct2Mycq5Q@mail.gmail.com обсуждение исходный текст |
| Ответ на | Memory Accounting v11 (Jeff Davis <pgsql@j-davis.com>) |
| Ответы |
Re: Memory Accounting v11
(Jeff Davis <pgsql@j-davis.com>)
Re: Memory Accounting v11 (Andres Freund <andres@anarazel.de>) |
| Список | pgsql-hackers |
On 15 June 2015 at 07:43, Jeff Davis <pgsql@j-davis.com> wrote:
This patch tracks memory usage (at the block level) for all memory
contexts. Individual palloc()s aren't tracked; only the new blocks
allocated to the memory context with malloc().
It also adds a new function, MemoryContextMemAllocated() which can
either retrieve the total allocated for the context, or it can recurse
and sum up the allocations for all subcontexts as well.
This is intended to be used by HashAgg in an upcoming patch that will
bound the memory and spill to disk.
Previous discussion here:
http://www.postgresql.org/message-id/1407012053.15301.53.camel@jeff-desktop
Previous concerns:
* There was a slowdown reported of around 1-3% (depending on the exact
version of the patch) on an IBM power machine when doing an index
rebuild. The results were fairly noisy for me, but it seemed to hold up.
See http://www.postgresql.org/message-id/CA
+Tgmobnu7XEn1gRdXnFo37P79bF=qLt46=37ajP3Cro9dBRaA@mail.gmail.com
* Adding a struct field to MemoryContextData may be bad for the CPU
caching behavior, and may be the cause of the above slowdown.
I've been looking at this patch and trying to reproduce the reported slowdown by using Tomas' function to try to exercise palloc() with minimal overhead of other code:
I also wanted to attempt to determine if the slowdown was due to the mem_allocated maintenance or if it was down to the struct size increasing.
I decided to test this just by simply commenting out all of the context->mem_allocated += / -= and just keep the mem_allocated field in the struct, but I've been really struggling to see any sort of performance decrease anywhere. I'm just getting too much variation in the test results to get any sort of idea.
I've attached a spreadsheet of the results I saw. It would be really good if Robert was able to test with the IBM PPC just with the extra field in the struct so we can see if the 1-3% lies there, or if it's in overhead of keeping mem_allocated up-to-date.
My main question here is: How sure are you that none of your intended use cases will need to call MemoryContextMemAllocated with recurse = true? I'd imagine if you ever have to use MemoryContextMemAllocated(ctx, true) then we'll all be sitting around with our stopwatches out to see if the call incurs too much of a performance penalty.
Other small issues:
+ oldblksize = block->endptr - ((char *)block);
+
block = (AllocBlock) realloc(block, blksize);
if (block == NULL)
return NULL;
+
+ context->mem_allocated += blksize - oldblksize;
+
Shouldn't you be setting oldblksize after the "if"? I'd imagine the realloc() failure is rare, but it just seems better to do it just before it's required and only when required.
+ if (recurse)
+ {
+ MemoryContext child = context->firstchild;
+ for (child = context->firstchild;
+ child != NULL;
Here there's a double assignment of "child".
***************
*** 632,637 **** AllocSetDelete(MemoryContext context)
--- 637,644 ----
{
AllocBlock next = block->next;
+ context->mem_allocated -= block->endptr - ((char *) block);
+
I might be mistaken here, but can't you just set context->mem_allocted = 0; after that loop?
Or maybe it would be an improvement to only do the decrement if MEMORY_CONTEXT_CHECKING is defined, then Assert() that mem_allocated == 0 after the loop...
One other thing that I don't remember seeing discussed would be to just store a List in each context which would store all of the mem_allocated's that need to be updated during each allocation and deallocation of memory. The list would be setup once when the context is created. When a child context is setup we'd just loop up the parent context chain and list_concat() all of the parent's lists onto the child's lists. Would this method add too much overhead when a context is deleted? Has this been explored already?
Regards
David Rowley
--
David Rowley http://www.2ndQuadrant.com/
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Michael PaquierДата:
Сообщение: Re: Support for N synchronous standby servers - take 2