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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
