Обсуждение: pgsql: jit: Re-allow JIT compilation of execGrouping.c hashtable compar
jit: Re-allow JIT compilation of execGrouping.c hashtable comparisons. In the course of 5567d12ce03, 356687bd8 and 317ffdfeaac, I changed BuildTupleHashTable[Ext]'s call to ExecBuildGroupingEqual to not pass in the parent node, but NULL. Which in turn prevents the tuple equality comparator from being JIT compiled. While that fixes bug #15486, it is not actually necessary after all of the above commits, as we don't re-build the comparator when using the new BuildTupleHashTableExt() interface (as the content of the hashtable are reset, but the TupleHashTable itself is not). Therefore re-allow jit compilation for callers that use BuildTupleHashTableExt with a separate context for "metadata" and content. As in the previous commit, there's ongoing work to make this easier to test to prevent such regressions in the future, but that infrastructure is not going to be backpatchable. The performance impact of not JIT compiling hashtable equality comparators can be substantial e.g. for aggregation queries that aggregate a lot of input rows to few output rows (when there are a lot of output groups, there will be fewer comparisons). Author: Andres Freund Discussion: https://postgr.es/m/20190927072053.njf6prdl3vb7y7qb@alap3.anarazel.de Backpatch: 11, just as 5567d12ce03 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ac88807f9b227ddcd92b8be9a053094837c1b99a Modified Files -------------- src/backend/executor/execGrouping.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
Andres Freund <andres@anarazel.de> writes:
> jit: Re-allow JIT compilation of execGrouping.c hashtable comparisons.
I've got to say that I'm not very comfortable with changing stuff
when we've got less than 24 hours to the 12.0 GA wrap. At this
point you're not going to get complete buildfarm feedback (notably,
not from CLOBBER_CACHE_ALWAYS animals). Unless it's a horrible
crasher bug, it'd be better to wait till post-tag and ship it in
12.1.
Personally, I've got patches I've been sitting on for several days
to put in after the wrap.
regards, tom lane
Hi, On September 29, 2019 4:34:14 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: >Andres Freund <andres@anarazel.de> writes: >> jit: Re-allow JIT compilation of execGrouping.c hashtable >comparisons. > >I've got to say that I'm not very comfortable with changing stuff >when we've got less than 24 hours to the 12.0 GA wrap. I asked for input on hackers, nobody replied... I noticed the l the speed difference when doing casual 11 vs 12 testing (withan old v11 build, leading me to discover both issues), so it seems quite likely that others would be affected too. > At this >point you're not going to get complete buildfarm feedback (notably, >not from CLOBBER_CACHE_ALWAYS animals). Unless it's a horrible >crasher bug, it'd be better to wait till post-tag and ship it in >12.1. There's no animal doing CCA or valgrind with JIT enabled anyway... And the changes have no meaningful effect outside of jit(which I plan to change soon in master, but that obviously doesn't matter). Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> writes:
> On September 29, 2019 4:34:14 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I've got to say that I'm not very comfortable with changing stuff
>> when we've got less than 24 hours to the 12.0 GA wrap.
> I asked for input on hackers, nobody replied...
Yeah, that was sixty-plus hours ago. I think onlookers said nothing
because they assumed you'd push on Friday, which would have left a
reasonable amount of time for buildfarm testing. Waiting this long
changes the calculus.
Anyway, it's probably going to be all right, but please pay attention
to the calendar when we're hard up against a release date. There
should be a very very good reason to push any code changes this late,
and I don't think "performance regression" qualifies.
regards, tom lane