Hi, Is there a plan to merge this patch in PG16?
Thanks, Muhammad ________________________________ From: Andres Freund <and...@anarazel.de> Sent: Saturday, July 15, 2023 6:56 PM To: pgsql-hack...@postgresql.org <pgsql-hack...@postgresql.org> Cc: Thomas Munro <thomas.mu...@gmail.com> Subject: Improve heapgetpage() performance, overhead from serializable Hi, Several loops which are important for query performance, like heapgetpage()'s loop over all tuples, have to call functions like HeapCheckForSerializableConflictOut() and PredicateLockTID() in every iteration. When serializable is not in use, all those functions do is to to return. But being situated in a different translation unit, the compiler can't inline (without LTO at least) the check whether serializability is needed. It's not just the function call overhead that's noticable, it's also that registers have to be spilled to the stack / reloaded from memory etc. On a freshly loaded pgbench scale 100, with turbo mode disabled, postgres pinned to one core. Parallel workers disabled to reduce noise. All times are the average of 15 executions with pgbench, in a newly started, but prewarmed postgres. SELECT * FROM pgbench_accounts OFFSET 10000000; HEAD: 397.977 removing the HeapCheckForSerializableConflictOut() from heapgetpage() (incorrect!), to establish the baseline of what serializable costs: 336.695 pulling out CheckForSerializableConflictOutNeeded() from HeapCheckForSerializableConflictOut() in heapgetpage(), and avoiding calling HeapCheckForSerializableConflictOut() in the loop: 339.742 moving the loop into a static inline function, marked as pg_always_inline, called with static arguments for always_visible, check_serializable: 326.546 marking the always_visible, !check_serializable case likely(): 322.249 removing TestForOldSnapshot() calls, which we pretty much already decided on: 312.987 FWIW, there's more we can do, with some hacky changes I got the time down to 273.261, but the tradeoffs start to be a bit more complicated. And 397->320ms for something as core as this, is imo worth considering on its own. Now, this just affects the sequential scan case. heap_hot_search_buffer() shares many of the same pathologies. I find it a bit harder to improve, because the compiler's code generation seems to switch between good / bad with changes that seems unrelated... I wonder why we haven't used PageIsAllVisible() in heap_hot_search_buffer() so far? Greetings, Andres Freund
