Re: JIT compiling expressions/deform + inlining prototype v2.0

From: Andres Freund <andres(at)anarazel(dot)de>
To: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: JIT compiling expressions/deform + inlining prototype v2.0
Date: 2017-09-04 20:52:04
Message-ID: 20170904205204.7gu43spe2nkq4hmh@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2017-09-04 20:01:03 +0300, Konstantin Knizhnik wrote:
> > I previously had an early prototype of JITing [1] expression evaluation
> > and tuple deforming. I've since then worked a lot on this.
> >
> > Here's an initial, not really pretty but functional, submission. This
> > supports all types of expressions, and tuples, and allows, albeit with
> > some drawbacks, inlining of builtin functions. Between the version at
> > [1] and this I'd done some work in c++, because that allowed to
> > experiment more with llvm, but I've now translated everything back.
> > Some features I'd to re-implement due to limitations of C API.
> >
> >
> > I've whacked this around quite heavily today, this likely has some new
> > bugs, sorry for that :(
>
> Can you please clarify the following fragment calculating attributes
> alignment:

Hi. That piece of code isn't particularly clear (and has a bug in the
submitted version), I'm revising it.

>
> /* compute what following columns are aligned to */
> + if (att->attlen < 0)
> + {
> + /* can't guarantee any alignment after varlen field */
> + attcuralign = -1;
> + }
> + else if (att->attnotnull && attcuralign >= 0)
> + {
> + Assert(att->attlen > 0);
> + attcuralign += att->attlen;
> + }
> + else if (att->attnotnull)
> + {
> + /*
> + * After a NOT NULL fixed-width column, alignment is
> + * guaranteed to be the minimum of the forced alignment and
> + * length. XXX
> + */
> + attcuralign = alignto + att->attlen;
> + Assert(attcuralign > 0);
> + }
> + else
> + {
> + //elog(LOG, "attnotnullreset: %d", attnum);
> + attcuralign = -1;
> + }
>
>
> I wonder why in this branch (att->attnotnull && attcuralign >= 0)
> we are not adding "alignto" and comment in the following branch else if
> (att->attnotnull)
> seems to be not related to this branch, because in this case attcuralign is
> expected to be less then zero wjhich means that previous attribute is varlen
> field.

Yea, I've changed that already, although it's currently added earlier,
because the alignment is needed before, to access the column correctly.
I've also made number of efficiency improvements, primarily to access
columns with an absolute offset if all preceding ones are fixed width
not null columns - that is quite noticeable performancewise.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2017-09-04 20:56:32 Re: pgbench - minor fix for meta command only scripts
Previous Message Fabien COELHO 2017-09-04 20:41:03 Re: Variable substitution in psql backtick expansion