From b84a6dafbf2bb921baee53c0c1aba7719ee38817 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 7 Nov 2018 11:08:45 -0800 Subject: [PATCH] Move EEOP_*_SYSVAR evaluation out of line. This mainly de-duplicates code. As evaluating a system variable isn't the hottest path and the current inline implementation ends up calling out to an external function anyway, this is OK from a performance POV. The main motivation for de-duplicating is the upcoming slot abstraction work, after which there's not guaranteed to be a HeapTuple backing the slot. Author: Andres Freund, Amit Khandekar Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de --- src/backend/executor/execExprInterp.c | 58 +++++++++------------------ src/backend/jit/llvm/llvmjit.c | 4 +- src/backend/jit/llvm/llvmjit_expr.c | 38 ++++-------------- src/backend/jit/llvm/llvmjit_types.c | 2 +- src/include/executor/execExpr.h | 2 + src/include/jit/llvmjit.h | 2 +- 6 files changed, 33 insertions(+), 73 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index c08df6f162..f7eac2a572 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -490,55 +490,19 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_CASE(EEOP_INNER_SYSVAR) { - int attnum = op->d.var.attnum; - Datum d; - - /* these asserts must match defenses in slot_getattr */ - Assert(innerslot->tts_tuple != NULL); - Assert(innerslot->tts_tuple != &(innerslot->tts_minhdr)); - - /* heap_getsysattr has sufficient defenses against bad attnums */ - d = heap_getsysattr(innerslot->tts_tuple, attnum, - innerslot->tts_tupleDescriptor, - op->resnull); - *op->resvalue = d; - + ExecEvalSysVar(state, op, econtext, innerslot); EEO_NEXT(); } EEO_CASE(EEOP_OUTER_SYSVAR) { - int attnum = op->d.var.attnum; - Datum d; - - /* these asserts must match defenses in slot_getattr */ - Assert(outerslot->tts_tuple != NULL); - Assert(outerslot->tts_tuple != &(outerslot->tts_minhdr)); - - /* heap_getsysattr has sufficient defenses against bad attnums */ - d = heap_getsysattr(outerslot->tts_tuple, attnum, - outerslot->tts_tupleDescriptor, - op->resnull); - *op->resvalue = d; - + ExecEvalSysVar(state, op, econtext, outerslot); EEO_NEXT(); } EEO_CASE(EEOP_SCAN_SYSVAR) { - int attnum = op->d.var.attnum; - Datum d; - - /* these asserts must match defenses in slot_getattr */ - Assert(scanslot->tts_tuple != NULL); - Assert(scanslot->tts_tuple != &(scanslot->tts_minhdr)); - - /* heap_getsysattr has sufficient defenses against bad attnums */ - d = heap_getsysattr(scanslot->tts_tuple, attnum, - scanslot->tts_tupleDescriptor, - op->resnull); - *op->resvalue = d; - + ExecEvalSysVar(state, op, econtext, scanslot); EEO_NEXT(); } @@ -4006,6 +3970,22 @@ ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext) *op->resnull = false; } +void +ExecEvalSysVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext, + TupleTableSlot *slot) +{ + bool success; + + /* slot_getsysattr has sufficient defenses against bad attnums */ + success = slot_getsysattr(slot, + op->d.var.attnum, + op->resvalue, + op->resnull); + /* this ought to be unreachable, but it's cheap enough to check */ + if (unlikely(!success)) + elog(ERROR, "failed to fetch attribute from slot"); +} + /* * Transition value has not been initialized. This is the first non-NULL input * value for a group. We use it as the initial value for transValue. diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index a543dd3711..168072afd2 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -81,9 +81,9 @@ LLVMValueRef FuncStrlen; LLVMValueRef FuncVarsizeAny; LLVMValueRef FuncSlotGetsomeattrs; LLVMValueRef FuncSlotGetmissingattrs; -LLVMValueRef FuncHeapGetsysattr; LLVMValueRef FuncMakeExpandedObjectReadOnlyInternal; LLVMValueRef FuncExecEvalArrayRefSubscript; +LLVMValueRef FuncExecEvalSysVar; LLVMValueRef FuncExecAggTransReparent; LLVMValueRef FuncExecAggInitGroup; @@ -822,9 +822,9 @@ llvm_create_types(void) FuncVarsizeAny = LLVMGetNamedFunction(mod, "varsize_any"); FuncSlotGetsomeattrs = LLVMGetNamedFunction(mod, "slot_getsomeattrs"); FuncSlotGetmissingattrs = LLVMGetNamedFunction(mod, "slot_getmissingattrs"); - FuncHeapGetsysattr = LLVMGetNamedFunction(mod, "heap_getsysattr"); FuncMakeExpandedObjectReadOnlyInternal = LLVMGetNamedFunction(mod, "MakeExpandedObjectReadOnlyInternal"); FuncExecEvalArrayRefSubscript = LLVMGetNamedFunction(mod, "ExecEvalArrayRefSubscript"); + FuncExecEvalSysVar = LLVMGetNamedFunction(mod, "ExecEvalSysVar"); FuncExecAggTransReparent = LLVMGetNamedFunction(mod, "ExecAggTransReparent"); FuncExecAggInitGroup = LLVMGetNamedFunction(mod, "ExecAggInitGroup"); diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 4225877478..13e981e564 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -406,13 +406,8 @@ llvm_compile_expr(ExprState *state) case EEOP_OUTER_SYSVAR: case EEOP_SCAN_SYSVAR: { - int attnum = op->d.var.attnum; - LLVMValueRef v_attnum; - LLVMValueRef v_tuple; - LLVMValueRef v_tupleDescriptor; - LLVMValueRef v_params[4]; - LLVMValueRef v_syscol; LLVMValueRef v_slot; + LLVMValueRef v_params[4]; if (opcode == EEOP_INNER_SYSVAR) v_slot = v_innerslot; @@ -421,31 +416,14 @@ llvm_compile_expr(ExprState *state) else v_slot = v_scanslot; - Assert(op->d.var.attnum < 0); - - v_tuple = l_load_struct_gep(b, v_slot, - FIELDNO_TUPLETABLESLOT_TUPLE, - "v.tuple"); + v_params[0] = v_state; + v_params[1] = l_ptr_const(op, l_ptr(StructExprEvalStep)); + v_params[2] = v_econtext; + v_params[3] = v_slot; - /* - * Could optimize this a bit for fixed descriptors, but - * this shouldn't be that critical a path. - */ - v_tupleDescriptor = - l_load_struct_gep(b, v_slot, - FIELDNO_TUPLETABLESLOT_TUPLEDESCRIPTOR, - "v.tupledesc"); - v_attnum = l_int32_const(attnum); - - v_params[0] = v_tuple; - v_params[1] = v_attnum; - v_params[2] = v_tupleDescriptor; - v_params[3] = v_resnullp; - v_syscol = LLVMBuildCall(b, - llvm_get_decl(mod, FuncHeapGetsysattr), - v_params, lengthof(v_params), - ""); - LLVMBuildStore(b, v_syscol, v_resvaluep); + LLVMBuildCall(b, + FuncExecEvalSysVar, + v_params, lengthof(v_params), ""); LLVMBuildBr(b, opblocks[i + 1]); break; diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index 58316a760d..855a6977ee 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -99,9 +99,9 @@ void *referenced_functions[] = varsize_any, slot_getsomeattrs, slot_getmissingattrs, - heap_getsysattr, MakeExpandedObjectReadOnlyInternal, ExecEvalArrayRefSubscript, + ExecEvalSysVar, ExecAggTransReparent, ExecAggInitGroup }; diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index 02af68f2c2..ac53935d70 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -734,6 +734,8 @@ extern void ExecEvalAlternativeSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext); +extern void ExecEvalSysVar(ExprState *state, ExprEvalStep *op, + ExprContext *econtext, TupleTableSlot *slot); extern void ExecAggInitGroup(AggState *aggstate, AggStatePerTrans pertrans, AggStatePerGroup pergroup); extern Datum ExecAggTransReparent(AggState *aggstate, AggStatePerTrans pertrans, diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h index c81cff8a35..f3ea249283 100644 --- a/src/include/jit/llvmjit.h +++ b/src/include/jit/llvmjit.h @@ -79,9 +79,9 @@ extern LLVMValueRef FuncStrlen; extern LLVMValueRef FuncVarsizeAny; extern LLVMValueRef FuncSlotGetsomeattrs; extern LLVMValueRef FuncSlotGetmissingattrs; -extern LLVMValueRef FuncHeapGetsysattr; extern LLVMValueRef FuncMakeExpandedObjectReadOnlyInternal; extern LLVMValueRef FuncExecEvalArrayRefSubscript; +extern LLVMValueRef FuncExecEvalSysVar; extern LLVMValueRef FuncExecAggTransReparent; extern LLVMValueRef FuncExecAggInitGroup; -- 2.40.0