Re: Rethinking MemoryContext creation

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Rethinking MemoryContext creation
Date: 2017-12-12 19:30:58
Message-ID: 18302.1513107058@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> I've not done any benchmarking on this yet, just confirmed that it
> compiles and passes check-world.

So once I'd done some benchmarking, I was a bit disappointed: I could
not measure any difference anymore in "pgbench -S", and poking into
other scenarios found cases that were actually slower, like

do $$
begin
for i in 1..10000000 loop
declare x int;
begin
x := 42;
exception when others then null;
end;
end loop;
end$$;

which went from about 12.4 seconds in HEAD to about 13.6 with my
v2 patch. When I looked into the reason, I found that my earlier
blithe pronouncement that "optimizing for the unused-context case
seems like the wrong thing" was too simple. In this example we
create and delete two memory contexts per loop (a subtransaction
context and an ExprContext) and neither of them receives any
palloc requests. Some of the contexts created in a "pgbench -S"
scenario are the same way. So in these examples, we were on the
losing side of the replace-a-palloc-with-a-malloc trade.

However, if we can predict which contexts are more likely not to
receive traffic, we can fix this by doing things the old way for
those contexts. I instrumented AllocSetDelete to log whether
the target context had received any requests in its lifespan,
and aggregated the reports over a run of the core regression tests.
I found these cases where there were significantly more reports
of "context was never used" than "context was used":

379 CurTransactionContext was never used
24 CurTransactionContext was used
66978 ExprContext was never used
17364 ExprContext was used
993 HashTableContext was never used
25 HashTableContext was used
88 Logical Replication Launcher sublist was never used
2 Logical Replication Launcher sublist was used
11139 SPI Exec was never used
2421 SPI Exec was used
36 SetOp was never used
2 SetOp was used
185 Statistics snapshot was never used
104 Subplan HashTable Context was never used
44 Subplan HashTable Context was used
148 Subplan HashTable Temp Context was never used
1481 Table function arguments was never used
45 Table function arguments was used
22 TableFunc per value context was never used
52 Unique was never used
4 Unique was used
137 WindowAgg Aggregates was never used
2 WindowAgg Aggregates was used
127 WindowAgg Partition was never used
12 WindowAgg Partition was used
288 _bt_pagedel was never used
14 _bt_pagedel was used
35 event trigger context was never used
3 event trigger context was used
38386 ginPlaceToPage temporary context was never used
3348 ginPlaceToPage temporary context was used
454 ident parser context was never used
229 tSRF function arguments was never used
46 tSRF function arguments was used

A lot of these probably aren't bothering with optimizing because they just
don't occur often enough to move the needle. (And in workloads where that
wasn't true, maybe the usage statistics would be different anyway.) But
it looked to me like CurTransactionContext, ExprContext, HashTableContext,
SPI Exec, and ginPlaceToPage temporary context would be worth doing it
the old way for.

Accordingly, attached is a v3 patch that adds another flag to
AllocSetContextCreateExtended telling it to do things the old way
with the context header in TopMemoryContext. (We can still optimize
context name copying as before.) This fixes the subtransaction case
I showed above, bringing it to 10.7 sec which is noticeably better
than HEAD. I now see maybe a 1 or 2 percent improvement in "pgbench -S",
which isn't much, but it turns out that that test case only involves
half a dozen context creations/deletions per transaction. So probably
we aren't going to get any noticeable movement on that benchmark, and
it'd be brighter to look for test cases where more contexts are involved.

regards, tom lane

Attachment Content-Type Size
rethink-memory-context-creation-3.patch text/x-diff 70.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-12-12 19:50:37 Re: Rethinking MemoryContext creation
Previous Message Andres Freund 2017-12-12 19:19:35 Re: Using ProcSignal to get memory context stats from a running backend