Hi,
Thank you for your replay and explanations.
My comments are inside.
On 04.12.2019 22:43, Andres Freund wrote:
Hi,
On 2019-11-25 18:09:29 +0300, Konstantin Knizhnik wrote:
I wonder why even at this query, which seems to be ideal use case for JIT,
we get such modest improvement?
I think there's a number of causes:
1) There's bottlenecks elsewhere:
- The order of sequential scan memory accesses is bad
https://www.postgresql.org/message-id/20161030073655.rfa6nvbyk4w2kkpk%40alap3.anarazel.de
In my experiments, fixing that yields larger JIT improvements,
because less time is spent stalling due to cache misses during
tuple deforming (needing the tuple's natts at the start prevents
out-of-order from hiding the relevant latency).
This is why I have implemented my own in-memory table access method.
It stores tuples in unpacked format so there should be no tuple
deforming overhead.
By the way if somebody is interested (mostly for experiments, I do not
thing that it in the current state it has some practival meaning)
my in-memory storage implementation is here:
https://github.com/postgrespro/postgresql.builtin_pool/tree/inmem_am
- The transition function for floating point aggregates is pretty
expensive. In particular, we compute the full youngs-cramer stuff
for sum/avg, even though they aren't actually needed there. This
has become measurably worse with
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=e954a727f0c8872bf5203186ad0f5312f6183746
In this case it's complicated enough apparently that the transition
functions are too expensive to inline.
- float4/8_accum use arrays to store the transition state. That's
noticably more expensive than just accessing a struct, partially
because more checks needs to be done. We really should move most,
if not all, aggregates that use array transition states to
"internal" type transition states. Probably with some reusable
helpers to make it easier to write serialization / deserialization
functions so we can continue to allow parallelism.
Yes, it is true.
Profile shows that packing/unpacking transition state takes substantial
amount of time.
But youngs-cramer stuff is not used for SUM aggregate! It is using
float4pl for accumulation and float4 as transaction type.
When I replace AVG with sum time of query execution with in-mem storage
is decreased from 6 seconds to 4 seconds.
But in VOPS improvement was even larger: 700 msec vs. 1865 msec. So the
gap in performance is even larger.
And profile is shows that aggregate infrastructure overhead disappears:
8.82% postgres postgres [.] ExecScan
4.60% postgres postgres [.] fetch_input_tuple
4.51% postgres postgres [.] inmem_getnextslot
3.48% postgres postgres [.] SeqNext
3.13% postgres postgres [.] ExecAgg
2.78% postgres postgres [.] MemoryContextReset
1.77% postgres perf-27660.map [.] 0x00007fcb81ae9032
1.52% postgres perf-27660.map [.] 0x00007fcb81ae9a4a
1.50% postgres perf-27660.map [.] 0x00007fcb81ae9fa2
1.49% postgres perf-27660.map [.] 0x00007fcb81ae9dcd
1.44% postgres perf-27660.map [.] 0x00007fcb81aea205
1.42% postgres perf-27660.map [.] 0x00007fcb81ae9072
1.31% postgres perf-27660.map [.] 0x00007fcb81ae9a56
1.22% postgres perf-27660.map [.] 0x00007fcb81ae9df1
1.22% postgres perf-27660.map [.] 0x00007fcb81aea225
1.21% postgres perf-27660.map [.] 0x00007fcb81ae93e6
1.21% postgres perf-27660.map [.] 0x00007fcb81ae9fae
1.19% postgres perf-27660.map [.] 0x00007fcb81ae9c83
1.12% postgres perf-27660.map [.] 0x00007fcb81ae9e5b
1.12% postgres perf-27660.map [.] 0x00007fcb81ae9c5f
1.05% postgres perf-27660.map [.] 0x00007fcb81ae9010
1.05% postgres perf-27660.map [.] 0x00007fcb81ae987b
As far as I understand positions in profile starting from 7-th are
corresponding to JIT code.
- The per-row overhead on lower levels of the query is
significant. E.g. in your profile the
HeapTupleSatisfiesVisibility() calls (you'd get largely rid of this
by freezing), and the hashtable overhead is quite noticable. JITing
expression eval doesn't fix that.
Once again: my in-memory storage doesn't perform visibility checks.
This is was the primary idea of my experiment: try to minimize per-row
storage overhead and check if JIT can provide performance
comparable with vectorized engine. Unfortunately the answer was
negative: the difference with VOPS is more than three times, while
difference between
standard table and in-memory table is less than 1.5.
...
2) The code generated for JIT isn't that good. In particular, the
external memory references included in the generated code limit the
optimization potential quite substantially. There's also quite some
(not just JIT) improvement potential related to the aggregation code,
simplifying the generated expressions.
See
https://www.postgresql.org/message-id/20191023163849.sosqbfs5yenocez3%40alap3.anarazel.de
for my attempt at improving the situation. It does measurably
improve the situation for Q1, while still leaving a lot of further
improvements to be done. You'd be more than welcome to review some
of that!
3) Plenty of crucial code is not JITed, even when expression
related. Most crucial for Q1 is the fact that the hash computation
for aggregates isn't JITed as a whole - when looking at hierarchical
profiles, we spend about 1/3 of the whole query time within
TupleHashTable*.
4) The currently required forming / deforming of tuples into minimal
tuples when storing them in the hashagg table is *expensive*.
We can address that partially by computing NOT NULL information for
the tupledesc used for the hashtable (which will make JITed tuple
deforming considerably faster, because it'll just be a reference to
an hardcoded offset).
We can also simplify the minimal tuple representation - historically
it looks the way it does now because we needed minimal tuples to be
largely compatible with heap tuples - but we don't anymore. Even just
removing the weird offset math we do for minimal tuples would be
beneficial, but I think we can do more than that.
Yes, this is the first think I have noticed. VOPS is calling hash
function only once per 64 rows - 64 times smaller than row storage.
This is why VOPS is 6 times faster on Q1 than vanilla postgres and 5
times than my in-memory storage.
And this is why I removed aggregation from Q1 and just calculates grand
aggregates.
VOPS provides 10x improvement of Q1.
My understanding of VOPS is that it ferries around more than one tuple
at a time. And avoids a lot of generic code paths. So that just doesn't
seem a meaningful comparison.
VOPS is just an example of vectorized executor.
It is possible to implement things which VOPS is performing using
customized types
using custom nodes as in Hubert Zhang prototype:
https://www.postgresql.org/message-id/flat/CAB0yrenxJ3FcmnLs8JqpEG3tzSZ%3DOL1MZBUh3v6dgH%2Bo70GTFA%40mail.gmail.com#df50bbf3610dc2f42cb9b54423a22111
In theory by elimination of interpretation overhead JIT should provide
performance comparable with vecrtorized executor.
I don't think that's true at all. Vectorized execution, which I assume
to mean dealing with more than one tuple at a time, is largely
orthogonal to the way expressions are evaluated. The reason that
vectorized execution is good is that it drastically increases cache
locality (by performing work that accesses related data, e.g. a buffer
page, in a tight loop, without a lot of other work happening inbetween),
that it increases the benefits of out of order execution (by removing
dependencies, as e.g. predicates for multiple tuples can be computed,
without a separate dependency on the result for each predicate
evaluation), etc.
JIT compiled expression evaluation cannot get you these benefits.
Yes, I know this arguments.
But please look here in Q1 and lineitem table projection we have just 4
float4 attributes and calculates 7 aggregates for them.
It seems to me that in this case CPU cache will be even more efficiently
using in case of horizontal calculation.
At least if you implement correspondent query in C, then version working
with array of struct will be almost two times
faster than version working with vertical arrays.
Note that just looking at a plain porfile, without injecting information
about the JITed code, will yield misleading results. Without the
additional information perf will not be able to group the instructions
of the JITed code sampled to a function, leading to them each being
listed separately.
If you enable jit_profiling_support, and measure with
perf record -k 1 -o /tmp/perf.data -p 22950
(optionally with --call-graph lbr)
you then can inject the information about JITed code:
perf inject -v --jit -i /tmp/perf.data -o /tmp/perf.jit.data
and look at the result of that with
perf report -i /tmp/perf.jit.data
Something is not working properly in my case:
root@knizhnik:~# perf record -k 1 -o /tmp/perf.data -p 7407
^C[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.452 MB /tmp/perf.data (11410 samples) ]
root@knizhnik:~# perf inject -v --jit -i /tmp/perf.data -o
/tmp/perf.jit.data
build id event received for [kernel.kallsyms]:
b1ef0f6204a7ec3f508b9e1536f73521c7b4b41a
build id event received for /home/knizhnik/postgresql/dist/bin/postgres:
8ef1a41e80f043a56778e265f5badb67f1441b61
build id event received for [vdso]: b13824592e1e837368d92991b72a19437dc86a27
Looking at the vmlinux_path (8 entries long)
symsrc__init: cannot get elf header.
Using /proc/kcore for kernel object code
Using /proc/kallsyms for symbols
Using CPUID GenuineIntel-6-3C
root@knizhnik:~# perf report -i /tmp/perf.jit.data
7.37% postgres postgres [.] ExecScan
7.23% postgres postgres [.] inmem_getnextslot
4.79% postgres postgres [.] fetch_input_tuple
4.07% postgres postgres [.] SeqNext
3.52% postgres postgres [.] ExecAgg
2.68% postgres postgres [.] MemoryContextReset
1.62% postgres perf-7407.map [.] 0x00007f4591c95f02
1.50% postgres perf-7407.map [.] 0x00007f4591c95d2d
...
The fact that ExecInterpExpr, tts_minimal_getsomeattrs show up
significantly suggests that you're running a slightly older build,
without a few bugfixes. Could that be true?
My forked my branch on your commit from 27 November
(ca266a069a20c32a8f0a1df982a5a67d9483bcb3).
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
