Re: Enhanced error message to include hint messages for redundant options error

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: Enhanced error message to include hint messages for redundant options error
Date: 2021-07-11 13:29:23
Message-ID: CALDaNm3EGS+cyDVc+g8O3pJtfMTvbT5SKygzoWvHY4q-b_a6UQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

.On Sun, Jul 11, 2021 at 2:57 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> On Sat, 10 Jul 2021 at 17:03, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > > Also, I don't think this function should be marked inline -- using a
> > > normal function ought to help make the compiled code smaller.
> >
> > inline instructs the compiler to attempt to embed the function content
> > into the calling code instead of executing an actual call. I think we
> > should keep it inline to reduce the function call.
>
> Hmm, I'd say that inline should generally be used sparingly, and only
> for small functions that are called very often, to avoid the function
> call overhead, and generate a faster and possibly smaller executable.
> (Though I think sometimes it can still be faster if the executable is
> larger.)
>
> In this case, it's a function that is only called under error
> conditions, so it's not commonly called, and we don't care so much
> about performance when we're about to throw an error.
>
> Also, if you look at an ereport() such as
>
> ereport(ERROR,
> errcode(ERRCODE_SYNTAX_ERROR),
> errmsg("conflicting or redundant options"),
> parser_errposition(pstate, defel->location)));
>
> This is a macro that's actually expanded into 5 separate function calls:
>
> - errstart() / errstart_cold()
> - errcode()
> - errmsg()
> - parser_errposition()
> - errfinish()
>
> so it's a non-trivial amount of code. Whereas, if it's not inlined, it
> becomes just one function call at each call-site, making for smaller,
> faster code in the typical case where an error is not being raised.
>
> Of course, it's possible the compiler might still decide to inline the
> function, if it thinks that's preferable. In some cases, we explicitly
> mark this type of function with pg_noinline, to avoid that, and reduce
> code bloat where it's used in lots of small, fast functions (see, for
> example, float_overflow_error()).
>
> In general though, I think inline and noinline should be reserved for
> special cases where they give a clear, measurable benefit, and that in
> general it's better to not mark the function and just let the compiler
> decide.

Ok, that makes sense. As this flow is mainly in the error part it is
ok. I will change it.

Regards,
Vignesh

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michail Nikolaev 2021-07-11 13:51:11 Re: Slow standby snapshot
Previous Message Fabien COELHO 2021-07-11 13:07:09 Re: pgbench logging broken by time logic changes