Thread: Defaulting to jit=on/off for v11
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
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
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
Attachment
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
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
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'thave an easy time to push a patch. I probably could push something tomorrow, but I'd have a hard time cleaning upif 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.
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'thave an easy time to push a patch. I probably could push something tomorrow, but I'd have a hard time cleaning
upif 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
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 therelease notes, mentioning that it has to be turned on. Thanks! Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
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
tothe release notes, mentioning that it has to be turned on.
> Thanks!
Roger, I'll have a go at it.
regards, tom lane
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 paragraphto 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
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