Hi,
On 2017-03-17 11:36:30 -0400, Tom Lane wrote:
> Having said all that, I think that 0001 is contributing very little to the
> goals of this patch set. Andres stated that he wanted it so as to drop
> some of the one-time checks that execQual.c currently does for Vars, but
> I'm not really convinced that we could do that safely even with these
> additional dependencies in place. Moreover, I really doubt that there's
> a horrible performance cost from including something like
>
> if (unlikely(op->first_execution))
> out_of_line_checking_subroutine(...);
> in the execution code for Vars.
But it actually does (well, for some relatively small value of horrible)
The issue is that op->first_execution is actually badly predictable,
because it will be set back/forth between executions of different
expressions (in the same plantree). Obviously you'll not notice if you
have a Var and then some expensive stuff, but it's noticeable for
cheap-ish expressions (say e.g. a single Var). So the branch prediction
often doesn't handle this gracefully - it also just expands the set of
to-be-tracked jumps.
If we had a decent way to actually check this during ExecInitExpr() (et
al), the whole discussion would be different - I'd be all expanding the
set of such checks even. But I couldn't find a decent way to get there
- when expressions are initialized we don't even get an ExprContext (not
to speak of valid slots), nor is parent->plan very helpful.
That said, it seems this is something that has to wait for a later
release, I'm putting back in similar logic as there was before (not a
branch, but change the opcode to a non-checking variant).
> And that certainly isn't adding any
> complexity for JIT compilation that we don't face anyway for other
> execution step types.
Obviously a if (op->first_execution) isn't an issue, it's actually only
doing the first time through that's not easily possible.
> So my recommendation is to drop 0001 and include the same one-time
> checks that execQual.c currently has as out-of-line one-time checks
> in the new code. We can revisit that later, but time grows short for
> v10. I would much rather have a solid version of 0004 and not 0001,
> than not have anything for v10 because we spent too much time dealing
> with adding new dependencies.
Doing that (+README).
Greetings,
Andres Freund
