[PATCH] heap_insert() and heap_update() optimization
Hello, hackers
I suggest the small attached patch that gives a bit of heap_insert() and heap_update() optimization
by reducing calls of BufferGetPage(buffer) into them.
I measured call time of these:
heap_insert(): avg origin 13394 ns, avg patched 12685 ns; perf increases +5.59%
heap_update(): avg origin 15728 ns, avg patched 13936 ns; perf increases +11.39%
This can be notable when there are handling many rows.
--
Regards,
Andrew K.
Attachments:
v1-00-heap_insert-and-heap_update_optimized.patchapplication/x-patch; name="=?UTF-8?B?djEtMDAtaGVhcF9pbnNlcnQtYW5kLWhlYXBfdXBkYXRlX29wdGltaXplZC5w?= =?UTF-8?B?YXRjaA==?="Download
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index ce8cdb9..8759996 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2467,6 +2467,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
HeapTuple heaptup;
Buffer buffer;
Buffer vmbuffer = InvalidBuffer;
+ Page page;
bool all_visible_cleared = false;
/*
@@ -2509,10 +2510,11 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
RelationPutHeapTuple(relation, buffer, heaptup,
(options & HEAP_INSERT_SPECULATIVE) != 0);
- if (PageIsAllVisible(BufferGetPage(buffer)))
+ page = BufferGetPage(buffer);
+ if (PageIsAllVisible(page))
{
all_visible_cleared = true;
- PageClearAllVisible(BufferGetPage(buffer));
+ PageClearAllVisible(page);
visibilitymap_clear(relation,
ItemPointerGetBlockNumber(&(heaptup->t_self)),
vmbuffer, VISIBILITYMAP_VALID_BITS);
@@ -3536,7 +3538,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
HeapTuple heaptup;
HeapTuple old_key_tuple = NULL;
bool old_key_copied = false;
- Page page;
+ Page page,
+ newbuf_page;
BlockNumber block;
MultiXactStatus mxact_status;
Buffer buffer,
@@ -4301,17 +4304,19 @@ l2:
oldtup.t_data->t_ctid = heaptup->t_self;
/* clear PD_ALL_VISIBLE flags, reset all visibilitymap bits */
- if (PageIsAllVisible(BufferGetPage(buffer)))
+ page = BufferGetPage(buffer);
+ if (PageIsAllVisible(page))
{
all_visible_cleared = true;
- PageClearAllVisible(BufferGetPage(buffer));
+ PageClearAllVisible(page);
visibilitymap_clear(relation, BufferGetBlockNumber(buffer),
vmbuffer, VISIBILITYMAP_VALID_BITS);
}
- if (newbuf != buffer && PageIsAllVisible(BufferGetPage(newbuf)))
+ newbuf_page = BufferGetPage(newbuf);
+ if (newbuf != buffer && PageIsAllVisible(newbuf_page))
{
all_visible_cleared_new = true;
- PageClearAllVisible(BufferGetPage(newbuf));
+ PageClearAllVisible(newbuf_page);
visibilitymap_clear(relation, BufferGetBlockNumber(newbuf),
vmbuffer_new, VISIBILITYMAP_VALID_BITS);
}
@@ -4342,9 +4347,9 @@ l2:
all_visible_cleared_new);
if (newbuf != buffer)
{
- PageSetLSN(BufferGetPage(newbuf), recptr);
+ PageSetLSN(newbuf_page, recptr);
}
- PageSetLSN(BufferGetPage(buffer), recptr);
+ PageSetLSN(page, recptr);
}
END_CRIT_SECTION();
Hi,
On 2018-10-16 11:28:17 +0300, Andrey Klychkov wrote:
I suggest the small attached patch that gives a bit of heap_insert() and heap_update() optimization
by reducing calls of BufferGetPage(buffer) into them.
I measured call time of these:
heap_insert(): avg origin 13394 ns, avg patched 12685 ns; perf increases +5.59%
heap_update(): avg origin 15728 ns, avg patched 13936 ns; perf increases +11.39%
This can be notable when there are handling many rows.
Interesting. That's with an optimized build, or an assertion build?
Wonder what precisely prevents the optimizer to recognize
BufferGetPage() with a constant argument will always be the same
result. I assume it's that it doesn't recognize that BufferBlocks can't
change across other function calls? Might also be the pointer math, or
the if block...
Wonder if we could force the compiler's hand by making BufferGetPage an
inline function and decorating it with __attribute__((pure)) or such.
I see little reason to not apply what you have here, but there's a lot
of other places that access buffers...
Greetings,
Andres Freund
Interesting. That's with an optimized build, or an assertion build?
Hello,
That was an optimized build.
However I've just done some extra time tests and didn't notice so significant difference as early.
Even more - avg origin 1272, avg patched 1303.
Maybe there was the autovacuum / analyze / checkpoint or something else that could influence on the yesterday tests.
Thanks a lot for explanation!
Вторник, 16 октября 2018, 22:57 +03:00 от Andres Freund <andres@anarazel.de>:
Hi,
On 2018-10-16 11:28:17 +0300, Andrey Klychkov wrote:
I suggest the small attached patch that gives a bit of heap_insert() and heap_update() optimization
by reducing calls of BufferGetPage(buffer) into them.
I measured call time of these:
heap_insert(): avg origin 13394 ns, avg patched 12685 ns; perf increases +5.59%
heap_update(): avg origin 15728 ns, avg patched 13936 ns; perf increases +11.39%
This can be notable when there are handling many rows.Interesting. That's with an optimized build, or an assertion build?
Wonder what precisely prevents the optimizer to recognize
BufferGetPage() with a constant argument will always be the same
result. I assume it's that it doesn't recognize that BufferBlocks can't
change across other function calls? Might also be the pointer math, or
the if block...Wonder if we could force the compiler's hand by making BufferGetPage an
inline function and decorating it with __attribute__((pure)) or such.I see little reason to not apply what you have here, but there's a lot
of other places that access buffers...Greetings,
Andres Freund
--
Regards,
Andrey Klychkov
Hi,
On 2018-10-17 09:48:19 +0300, Andrey Klychkov wrote:
�Interesting. That's with an optimized build, or an assertion build?
Hello,
That was an optimized build.However I've just done some extra time tests and didn't notice so significant difference as early.
Even more - avg origin�1272, avg patched�1303.Maybe there was the autovacuum / analyze / checkpoint or something else that could influence on the yesterday tests.
Probably worth looking at the generated code. I can see some difference,
but what you measured seemed pretty large.
Greetings,
Andres Freund