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

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: vignesh C <vignesh21(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 09:27:00
Message-ID: CAEZATCVn7hJm9=C0z2ttkmGRtZrXtrJZWvJ7ryMC=aWuXx14Qw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dean Rasheed 2021-07-11 09:52:56 Re: Enhanced error message to include hint messages for redundant options error
Previous Message Thomas Munro 2021-07-11 08:16:56 Re: pgbench logging broken by time logic changes