Re: Query is over 2x slower with jit=on
Jonathan S. Katz <jkatz@postgresql.org>
From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>,
Alvaro Herrera <alvherre@2ndquadrant.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Andreas Joseph Krogh <andreas@visena.com>
Date: 2018-08-22T16:36:34Z
Lists: pgsql-hackers
> On Aug 22, 2018, at 12:12 PM, Andres Freund <andres@anarazel.de> wrote: > > Hi, > > On 2018-04-18 18:37:30 -0400, Robert Haas wrote: >> On Wed, Apr 18, 2018 at 3:29 PM, Andres Freund <andres@anarazel.de> wrote: >>> Not convinced that that is true - the issue is more likely that JIT work in workers is counted as execute time... Gotta add that somehow, not sure what the best way would be. >> >> Oh, that does seem like something that should be fixed. If that's >> what is happening here, it's bound to confuse a lot of people. >> Probably you need to add some code to >> ExecParallelRetrieveInstrumentation. > > I had lost track of this, and we unfortunately hadn't added an open item > back then. I think we should add it now? > > RMT (with me recused), do you think we should accept the new code fixing > this would entail? And thus that this should be an open item? It's > arguably a new feature, although I don't find that a terribly convincing > position. Reviewed the earlier discussion. I can argue this both ways. As it stands right now it seems like we will be defaulting “jit = off” which means there will only be so many people who will be using jit in PG11, and as such we could provide the patch for 12. However, for the people who do enable JIT in PG11, this is a scenario that would cause the query to perform in an unexpected way that is no fault of their own, which one would argue is a bug. I have not tried to reproduce it myself, but from looking at the explain plans from Andreas, I’m confident I could craft a moderately complex query that could demonstrate the performance degradation with jit = on. While adding the code may constitute being a new feature, it is a feature that would prevent user frustration on something that we are highlighting as a “major feature” of PG11, even if it’s not enabled by default. What I would you Andres is how complex will the patch be and how much time will it take? As such I would +1 it for open items right now unless it seems like this could take a significant of time to come up with a proper patch, in which case I would reconsider, but at least with it on open items we would be able to continuously review. Thanks, Jonathan
Commits
-
Fix issues around EXPLAIN with JIT.
- e97c4d967ba5 11.0 landed
- c03c1449c092 12.0 landed
-
Collect JIT instrumentation from workers.
- e63441c3f5ca 11.0 landed
- 33001fd7a707 12.0 landed
-
Make EXPLAIN output for JIT compilation more dense.
- 6859bd2632d8 11.0 landed
- 52050ad8ebec 12.0 landed
-
Save/restore SPI's global variables in SPI_connect() and SPI_finish().
- 825f10fbda7a 11.0 cited
-
Rationalize handling of single and double quotes in bootstrap data.
- 55d26ff638f0 11.0 cited