Horton

Thread

Commits

  1. In v11, disable JIT by default (it's still enabled by default in HEAD).

  1. Defaulting to jit=on/off for v11

    Andres Freund <andres@anarazel.de> — 2018-09-14T22:26:57Z

    Hi,

I kind of thought I'd sent out this mail Friday morning, but apparently
I forgot to hit send, before accidentally releasing my laptop's battery
while running :/

We'd agreed a while ago, when JIT compilation landed, to decide close to
the release whether JIT compilation should be enabled or disabled by
default. It seems the time for that has come.


I think there's several angles to this:

* Enabling by default will give far greater exposure, making it much
  more likely that we'll fix bugs earlier, meaning fewer people will be
  caught later when they already thought everything should be perfectly
  stable.  We'll want to enable the feature by default at some point, so
  that'd be good way to get there fast.

* There's a number of cases where JIT compilation triggers over-eagerly:
  In particular when costs are widely off (e.g. in the regression tests
  due to an un-analyzed table), or when the majority of the cost is
  incurred from things like enable_*=off.  That can cause slowdowns.  If
  we had caching of JITed code, that'd not be that bad, but we don't.

* There's a substantial risk that the JIT feature has bugs - it was
  largely written by a single person (i.e. me), nontrivial and there
  definitely have been features that have been more widely reviewed.

* Disabling it by default will prevent people from "automatically"
  benefiting.


I can see basically three sensible routes to go for v11 (before we
improve further):

1) Leave it enabled, as currently.

2) Disable it by default in v11, leave it enabled in master.

3) Increase the costs substantially, so it triggers in far fewer
   cases. Those would be too high for cases that want to benefit fully,
   but would reduce the risk - although it also probably makes it
   harder to easily hit problematic cases.


I personally can see reasons to go with either of these, and don't have
a real preference myself.


Comments?


Greetings,

Andres Freund

  2. Re: Defaulting to jit=on/off for v11

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-14T22:32:03Z

    Andres Freund <andres@anarazel.de> writes:
> I can see basically three sensible routes to go for v11 (before we
> improve further):

> 1) Leave it enabled, as currently.

> 2) Disable it by default in v11, leave it enabled in master.

> 3) Increase the costs substantially, so it triggers in far fewer
>    cases. Those would be too high for cases that want to benefit fully,
>    but would reduce the risk - although it also probably makes it
>    harder to easily hit problematic cases.

I'd go with #2, personally.  It does seem that the costing needs work,
but it's not clear to me that we know what to change, so it's kinda
late to propose #3 for v11.

			regards, tom lane

  3. Re: Defaulting to jit=on/off for v11

    Jonathan S. Katz <jkatz@postgresql.org> — 2018-09-14T22:47:56Z

    On 9/14/18 6:32 PM, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> I can see basically three sensible routes to go for v11 (before we
>> improve further):
> 
>> 1) Leave it enabled, as currently.
> 
>> 2) Disable it by default in v11, leave it enabled in master.
> 
>> 3) Increase the costs substantially, so it triggers in far fewer
>>    cases. Those would be too high for cases that want to benefit fully,
>>    but would reduce the risk - although it also probably makes it
>>    harder to easily hit problematic cases.
> 
> I'd go with #2, personally.  It does seem that the costing needs work,
> but it's not clear to me that we know what to change, so it's kinda
> late to propose #3 for v11.

First, I'm going to say I'm a huge fan of this feature and I'm excited
for the ongoing work.

From doing some testing, I hit a substantial performance loss on a query
that would be considered part of the critical path of a system. I'm sure
getting the costing right would help, but also that query did contain
expressions that could benefit from JIT compilation. My concern would be
 that other apps have queries that would make them start to under perform.

The nice thing is you can enable jit on a per query basis if needed -
more work for the user, but I think safer in the first release than
having it on by default.

+1 to option #2.

Jonathan

  4. Re: Defaulting to jit=on/off for v11

    Peter Geoghegan <pg@bowt.ie> — 2018-09-15T00:18:44Z

    On Fri, Sep 14, 2018 at 3:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I'd go with #2, personally.  It does seem that the costing needs work,
> but it's not clear to me that we know what to change, so it's kinda
> late to propose #3 for v11.

+1. I also favor option #2.

-- 
Peter Geoghegan

  5. Re: Defaulting to jit=on/off for v11

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-09-15T14:26:17Z

    
On 09/14/2018 08:18 PM, Peter Geoghegan wrote:
> On Fri, Sep 14, 2018 at 3:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd go with #2, personally.  It does seem that the costing needs work,
>> but it's not clear to me that we know what to change, so it's kinda
>> late to propose #3 for v11.
> +1. I also favor option #2.
>

+ about 0.8. I hope we do get some good field testing if it, though.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: Defaulting to jit=on/off for v11

    Andres Freund <andres@anarazel.de> — 2018-09-15T18:07:22Z

    
On September 15, 2018 8:26:17 AM MDT, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
>
>
>On 09/14/2018 08:18 PM, Peter Geoghegan wrote:
>> On Fri, Sep 14, 2018 at 3:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I'd go with #2, personally.  It does seem that the costing needs
>work,
>>> but it's not clear to me that we know what to change, so it's kinda
>>> late to propose #3 for v11.
>> +1. I also favor option #2.
>>
>
>+ about 0.8. I hope we do get some good field testing if it, though.

Cool, sounds we have agreement. I'll try to come up with a patch.  I'm [un]fortunately hiking till Wednesday, so I won't have an easy time to push a patch.   I probably could push something tomorrow, but I'd have a hard time cleaning up if needed.  Does anybody feel we should have that in Mondays release?

Andres

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  7. Re: Defaulting to jit=on/off for v11

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-15T18:14:07Z

    Andres Freund <andres@anarazel.de> writes:
> Cool, sounds we have agreement. I'll try to come up with a patch.  I'm [un]fortunately hiking till Wednesday, so I won't have an easy time to push a patch.   I probably could push something tomorrow, but I'd have a hard time cleaning up if needed.  Does anybody feel we should have that in Mondays release?

Recent discussions among the release team are pointing to the idea
that Monday's wrap could be 11rc1, not 11beta4, if we get this done
beforehand.  So I'd kind of like to see it done.

Is there more that has to be done than switching the GUC's default
value and adjusting docs?  I could probably make it happen if there's
not any hidden stuff to worry about.

			regards, tom lane

  8. Re: Defaulting to jit=on/off for v11

    Andres Freund <andres@anarazel.de> — 2018-09-15T18:17:24Z

    
Hi,

On September 15, 2018 12:14:07 PM MDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>Andres Freund <andres@anarazel.de> writes:
>> Cool, sounds we have agreement. I'll try to come up with a patch. 
>I'm [un]fortunately hiking till Wednesday, so I won't have an easy time
>to push a patch.   I probably could push something tomorrow, but I'd
>have a hard time cleaning up if needed.  Does anybody feel we should
>have that in Mondays release?
>
>Recent discussions among the release team are pointing to the idea
>that Monday's wrap could be 11rc1, not 11beta4, if we get this done
>beforehand.  So I'd kind of like to see it done.

Yes, that's why I'm asking...


>Is there more that has to be done than switching the GUC's default
>value and adjusting docs?  I could probably make it happen if there's
>not any hidden stuff to worry about.

No, that should be all.  I'll adapt the llvm animals to force the GUC to on. I think it'd be good to add a paragraph to the release notes, mentioning that it has to be turned on.

Thanks!

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  9. Re: Defaulting to jit=on/off for v11

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-15T18:19:55Z

    Andres Freund <andres@anarazel.de> writes:
> On September 15, 2018 12:14:07 PM MDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Is there more that has to be done than switching the GUC's default
>> value and adjusting docs?  I could probably make it happen if there's
>> not any hidden stuff to worry about.

> No, that should be all.  I'll adapt the llvm animals to force the GUC to on. I think it'd be good to add a paragraph to the release notes, mentioning that it has to be turned on.

> Thanks!

Roger, I'll have a go at it.

			regards, tom lane

  10. Re: Defaulting to jit=on/off for v11

    Andres Freund <andres@anarazel.de> — 2018-09-15T18:41:58Z

    On 2018-09-15 14:19:55 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On September 15, 2018 12:14:07 PM MDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Is there more that has to be done than switching the GUC's default
> >> value and adjusting docs?  I could probably make it happen if there's
> >> not any hidden stuff to worry about.
> 
> > No, that should be all.  I'll adapt the llvm animals to force the GUC to on. I think it'd be good to add a paragraph to the release notes, mentioning that it has to be turned on.
> 
> > Thanks!
> 
> Roger, I'll have a go at it.

Thanks!  Just managed to log into the machine hosting my llvm testing
buildfarm animals and set jit=1 for those. So we shouldn't loose too
much coverage.  I'll email the folks that have LLVM enabled in the BF,
to also do so.

Greetings,

Andres Freund

  11. Re: Defaulting to jit=on/off for v11

    Andres Freund <andres@anarazel.de> — 2018-09-15T18:59:11Z

    Hi,

On 2018-09-15 11:41:58 -0700, Andres Freund wrote:
> I'll email the folks that have LLVM enabled in the BF, to also do so.

Done so. I'll try to check-in again tonight, but while likely, it's not
certain there'll be reception.

- Andres