From 7faa36dbac5ef6ccae82887eca7d0439352fc3ea Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Tue, 30 Jan 2024 22:26:36 -0300 Subject: [PATCH 1/2] Improve performance of external (UDR) functions. --- examples/udr/Functions.cpp | 3 +- src/dsql/ExprNodes.cpp | 18 +- src/dsql/Nodes.h | 2 +- src/jrd/ExtEngineManager.cpp | 566 +++++++++++++++++++++++------------ src/jrd/ExtEngineManager.h | 36 ++- src/jrd/Function.h | 17 +- src/jrd/exe.cpp | 24 +- src/jrd/exe.h | 5 +- src/jrd/exe_proto.h | 1 + src/jrd/jrd.h | 18 +- 10 files changed, 447 insertions(+), 243 deletions(-) diff --git a/examples/udr/Functions.cpp b/examples/udr/Functions.cpp index 8c77e241565..51a3650702e 100644 --- a/examples/udr/Functions.cpp +++ b/examples/udr/Functions.cpp @@ -128,8 +128,7 @@ FB_UDR_BEGIN_FUNCTION(sum_args) // Get a reference to the return value. ISC_LONG& ret = *(ISC_LONG*) (out + outOffset); - // The return value is automatically initialized to 0. - ///ret = 0; + ret = 0; for (unsigned i = 0; i < inCount; ++i) { diff --git a/src/dsql/ExprNodes.cpp b/src/dsql/ExprNodes.cpp index 18b915621ae..27083cf0fc6 100644 --- a/src/dsql/ExprNodes.cpp +++ b/src/dsql/ExprNodes.cpp @@ -13051,7 +13051,7 @@ DmlNode* UdfCallNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch* if (argCount > node->function->fun_inputs) mismatchStatus << Arg::Gds(isc_wronumarg); - for (auto pos = 0; pos < positionalArgCount; ++pos) + for (auto pos = 0u; pos < positionalArgCount; ++pos) { if (pos < node->function->fun_inputs) { @@ -13505,12 +13505,20 @@ dsc* UdfCallNode::execute(thread_db* tdbb, Request* request) const funcRequest->setGmtTimeStamp(request->getGmtTimeStamp()); - EXE_start(tdbb, funcRequest, transaction); + if (function->fun_external) + { + function->fun_external->execute( + tdbb, funcRequest, transaction, inMsgLength, inMsg, outMsgLength, outMsg); + } + else + { + EXE_start(tdbb, funcRequest, transaction); - if (inMsgLength != 0) - EXE_send(tdbb, funcRequest, 0, inMsgLength, inMsg); + if (inMsgLength != 0) + EXE_send(tdbb, funcRequest, 0, inMsgLength, inMsg); - EXE_receive(tdbb, funcRequest, 1, outMsgLength, outMsg); + EXE_receive(tdbb, funcRequest, 1, outMsgLength, outMsg); + } // Clean up all savepoints started during execution of the function diff --git a/src/dsql/Nodes.h b/src/dsql/Nodes.h index 4849389876c..5bbb59a4465 100644 --- a/src/dsql/Nodes.h +++ b/src/dsql/Nodes.h @@ -1464,7 +1464,7 @@ class StmtNode : public DmlNode TYPE_TRUNCATE_LOCAL_TABLE, TYPE_UPDATE_OR_INSERT, - TYPE_EXT_INIT_PARAMETER, + TYPE_EXT_INIT_PARAMETERS, TYPE_EXT_TRIGGER }; diff --git a/src/jrd/ExtEngineManager.cpp b/src/jrd/ExtEngineManager.cpp index 992bcf4d29e..b5319e214ca 100644 --- a/src/jrd/ExtEngineManager.cpp +++ b/src/jrd/ExtEngineManager.cpp @@ -39,6 +39,7 @@ #include "../jrd/cmp_proto.h" #include "../jrd/cvt_proto.h" #include "../jrd/evl_proto.h" +#include "../jrd/exe_proto.h" #include "../jrd/intl_proto.h" #include "../jrd/met_proto.h" #include "../jrd/mov_proto.h" @@ -48,6 +49,7 @@ #include "../jrd/SystemPackages.h" #include "../common/isc_proto.h" #include "../common/classes/auto.h" +#include "../common/classes/fb_pair.h" #include "../common/classes/fb_string.h" #include "../common/classes/init.h" #include "../common/classes/objects_array.h" @@ -65,12 +67,65 @@ static EngineCheckout::Type checkoutType(IExternalEngine* engine); namespace { + // Compare two formats for equivalence, excluding fmt_defaults field. + bool sameFormats(const Format* fmt1, const Format* fmt2) + { + return fmt1->fmt_length == fmt2->fmt_length && + fmt1->fmt_count == fmt2->fmt_count && + fmt1->fmt_version == fmt2->fmt_version && + fmt1->fmt_desc == fmt2->fmt_desc; + } + + // Copy message between different formats. + void copyMessage(thread_db* tdbb, + const Format* srcFormat, UCHAR* srcMsg, + const Format* dstFormat, UCHAR* dstMsg) + { + fb_assert(srcFormat->fmt_desc.getCount() == dstFormat->fmt_desc.getCount()); + + const auto srcDescEnd = srcFormat->fmt_desc.begin() + (srcFormat->fmt_desc.getCount() / 2 * 2); + auto srcDescIt = srcFormat->fmt_desc.begin(); + auto dstDescIt = dstFormat->fmt_desc.begin(); + + while (srcDescIt < srcDescEnd) + { + fb_assert(srcDescIt[1].dsc_dtype == dtype_short); + fb_assert(dstDescIt[1].dsc_dtype == dtype_short); + + const auto srcArgOffset = (IPTR) srcDescIt[0].dsc_address; + const auto srcNullOffset = (IPTR) srcDescIt[1].dsc_address; + const auto srcNullPtr = reinterpret_cast(srcMsg + srcNullOffset); + + const auto dstArgOffset = (IPTR) dstDescIt[0].dsc_address; + const auto dstNullOffset = (IPTR) dstDescIt[1].dsc_address; + const auto dstNullPtr = reinterpret_cast(dstMsg + dstNullOffset); + + if (!*srcNullPtr) + { + dsc srcDesc = srcDescIt[0]; + srcDesc.dsc_address = srcMsg + srcArgOffset; + + dsc dstDesc = dstDescIt[0]; + dstDesc.dsc_address = dstMsg + dstArgOffset; + + MOV_move(tdbb, &srcDesc, &dstDesc); + + *dstNullPtr = 0; + } + else + *dstNullPtr = -1; + + srcDescIt += 2; + dstDescIt += 2; + } + } + // Internal message node. class IntMessageNode : public MessageNode { public: IntMessageNode(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, USHORT message, - Array >& aParameters, const Format* aFormat) + Array>& aParameters, const Format* aFormat) : MessageNode(pool), parameters(aParameters), format(aFormat) @@ -78,8 +133,8 @@ namespace setup(tdbb, csb, message, format->fmt_count); } - virtual USHORT setupDesc(thread_db* tdbb, CompilerScratch* csb, USHORT index, - dsc* desc, ItemInfo* itemInfo) + USHORT setupDesc(thread_db* tdbb, CompilerScratch* csb, USHORT index, + dsc* desc, ItemInfo* itemInfo) override { *desc = format->fmt_desc[index]; @@ -115,8 +170,8 @@ namespace } public: - Array >& parameters; - const Format* format; + Array>& parameters; + const Format* const format; }; // External message node. @@ -130,14 +185,14 @@ namespace setup(tdbb, csb, message, format->fmt_count); } - virtual USHORT setupDesc(thread_db* tdbb, CompilerScratch* csb, USHORT index, - dsc* desc, ItemInfo* itemInfo) + USHORT setupDesc(thread_db* tdbb, CompilerScratch* csb, USHORT index, + dsc* desc, ItemInfo* itemInfo) override { *desc = format->fmt_desc[index]; return type_alignments[desc->dsc_dtype]; } - virtual const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* exeState) const + const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* exeState) const override { if (request->req_operation == Request::req_evaluate) { @@ -150,99 +205,103 @@ namespace } public: - const Format* format; + const Format* const format; }; // Initialize output parameters with their domains default value or NULL. // Kind of blr_init_variable, but for parameters. - class InitParameterNode final : public TypedNode + class InitParametersNode final : public TypedNode { public: - InitParameterNode(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, - Array >& parameters, MessageNode* aMessage, USHORT aArgNumber) - : TypedNode(pool), - message(aMessage), - argNumber(aArgNumber) + InitParametersNode(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, + Array>& parameters, MessageNode* aMessage) + : TypedNode(pool), + message(aMessage) { - Parameter* parameter = parameters[argNumber / 2]; - defaultValueNode = NULL; + // Iterate over the format items, except the EOF item. + const unsigned paramCount = message->format->fmt_count / 2; - if (parameter->prm_mechanism != prm_mech_type_of && - !fb_utils::implicit_domain(parameter->prm_field_source.c_str())) + defaultValuesNode = FB_NEW_POOL(pool) ValueListNode(pool, paramCount); + + for (unsigned paramIndex = 0; paramIndex < paramCount; ++paramIndex) { - MetaNamePair namePair(parameter->prm_field_source, ""); + const auto parameter = parameters[paramIndex]; + + if (parameter->prm_mechanism != prm_mech_type_of && + !fb_utils::implicit_domain(parameter->prm_field_source.c_str())) + { + MetaNamePair namePair(parameter->prm_field_source, ""); - FieldInfo fieldInfo; - bool exist = csb->csb_map_field_info.get(namePair, fieldInfo); + FieldInfo fieldInfo; + bool exist = csb->csb_map_field_info.get(namePair, fieldInfo); - if (exist && fieldInfo.defaultValue) - defaultValueNode = CMP_clone_node(tdbb, csb, fieldInfo.defaultValue); + if (exist && fieldInfo.defaultValue) + defaultValuesNode->items[paramIndex] = CMP_clone_node(tdbb, csb, fieldInfo.defaultValue); + } } } - string internalPrint(NodePrinter& printer) const + string internalPrint(NodePrinter& printer) const override { StmtNode::internalPrint(printer); NODE_PRINT(printer, message); - NODE_PRINT(printer, argNumber); - NODE_PRINT(printer, defaultValueNode); + NODE_PRINT(printer, defaultValuesNode); - return "InitParameterNode"; + return "InitParametersNode"; } - void genBlr(DsqlCompilerScratch* /*dsqlScratch*/) + void genBlr(DsqlCompilerScratch* /*dsqlScratch*/) override { } - InitParameterNode* pass1(thread_db* tdbb, CompilerScratch* csb) + InitParametersNode* pass1(thread_db* tdbb, CompilerScratch* csb) override { - doPass1(tdbb, csb, &defaultValueNode); + doPass1(tdbb, csb, &defaultValuesNode); return this; } - InitParameterNode* pass2(thread_db* tdbb, CompilerScratch* csb) + InitParametersNode* pass2(thread_db* tdbb, CompilerScratch* csb) override { - ExprNode::doPass2(tdbb, csb, &defaultValueNode); + ExprNode::doPass2(tdbb, csb, &defaultValuesNode); return this; } - const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* /*exeState*/) const + const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* /*exeState*/) const override { if (request->req_operation == Request::req_evaluate) { - dsc* defaultDesc = NULL; + const auto msg = request->getImpure(message->impureOffset); + const auto paramCount = defaultValuesNode->items.getCount(); - if (defaultValueNode) + for (unsigned paramIndex = 0; paramIndex < paramCount; ++paramIndex) { - defaultDesc = EVL_expr(tdbb, request, defaultValueNode); + const auto defaultValueNode = defaultValuesNode->items[paramIndex]; + dsc* defaultDesc = nullptr; - if (request->req_flags & req_null) - defaultDesc = NULL; - } - - if (defaultDesc) - { - // Initialize the value. The null flag is already initialized to not-null - // by the ExtMessageNode. + if (defaultValueNode) + { + defaultDesc = EVL_expr(tdbb, request, defaultValueNode); - dsc desc = message->format->fmt_desc[argNumber]; - desc.dsc_address = request->getImpure( - message->impureOffset + (IPTR) desc.dsc_address); + if (request->req_flags & req_null) + defaultDesc = nullptr; + } - MOV_move(tdbb, defaultDesc, &desc); - } - else - { - SSHORT tempValue = -1; - dsc temp; - temp.makeShort(0, &tempValue); + const auto formatIndex = paramIndex * 2; + const auto& nullDesc = message->format->fmt_desc[formatIndex + 1]; + fb_assert(nullDesc.dsc_dtype == dtype_short); - dsc desc = message->format->fmt_desc[argNumber + 1]; - desc.dsc_address = request->getImpure( - message->impureOffset + (IPTR) desc.dsc_address); + if (defaultDesc) + { + // Initialize the value. The null flag is already initialized to not-null. + fb_assert(*(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) != -1); - MOV_move(tdbb, &temp, &desc); + dsc desc = message->format->fmt_desc[formatIndex]; + desc.dsc_address = msg + (IPTR) desc.dsc_address; + MOV_move(tdbb, defaultDesc, &desc); + } + else + *(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) = -1; } request->req_operation = Request::req_return; @@ -252,27 +311,8 @@ namespace } private: - MessageNode* message; - USHORT argNumber; - ValueExprNode* defaultValueNode; - }; - - // Output parameters initialization. - class InitOutputNode : public CompoundStmtNode - { - public: - InitOutputNode(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, - Array >& parameters, MessageNode* message) - : CompoundStmtNode(pool) - { - // Iterate over the format items, except the EOF item. - for (USHORT i = 0; i < (message->format->fmt_count / 2) * 2; i += 2) - { - InitParameterNode* init = FB_NEW_POOL(pool) InitParameterNode( - tdbb, pool, csb, parameters, message, i); - statements.add(init); - } - } + MessageNode* const message; + ValueListNode* defaultValuesNode; }; // Move parameters from a message to another, validating theirs values. @@ -285,14 +325,14 @@ namespace checkMessageEof(aCheckMessageEof) { // Iterate over the format items, except the EOF item. - for (USHORT i = 0; i < (fromMessage->format->fmt_count / 2) * 2; i += 2) + for (unsigned i = 0; i < (fromMessage->format->fmt_count / 2) * 2; i += 2) { - ParameterNode* flag = FB_NEW_POOL(pool) ParameterNode(pool); + auto flag = FB_NEW_POOL(pool) ParameterNode(pool); flag->messageNumber = fromMessage->messageNumber; flag->message = fromMessage; flag->argNumber = i + 1; - ParameterNode* param = FB_NEW_POOL(pool) ParameterNode(pool); + auto param = FB_NEW_POOL(pool) ParameterNode(pool); param->messageNumber = fromMessage->messageNumber; param->message = fromMessage; param->argNumber = i; @@ -337,38 +377,6 @@ namespace MessageNode* checkMessageEof; }; - // External function node. - class ExtFunctionNode : public SuspendNode - { - public: - ExtFunctionNode(MemoryPool& pool, const MessageNode* aExtInMessageNode, const MessageNode* aExtOutMessageNode, - const ExtEngineManager::Function* aFunction) - : SuspendNode(pool), - extInMessageNode(aExtInMessageNode), - extOutMessageNode(aExtOutMessageNode), - function(aFunction) - { - } - - virtual const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* exeState) const - { - if (request->req_operation == Request::req_evaluate) - { - UCHAR* inMsg = extInMessageNode ? request->getImpure(extInMessageNode->impureOffset) : NULL; - UCHAR* outMsg = request->getImpure(extOutMessageNode->impureOffset); - - function->execute(tdbb, inMsg, outMsg); - } - - return SuspendNode::execute(tdbb, request, exeState); - } - - private: - const MessageNode* extInMessageNode; - const MessageNode* extOutMessageNode; - const ExtEngineManager::Function* function; - }; - // External procedure node. class ExtProcedureNode : public CompoundStmtNode { @@ -390,7 +398,7 @@ namespace statements.add(FB_NEW_POOL(pool) StallNode(pool)); } - virtual const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* exeState) const + const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* exeState) const override { impure_state* const impure = request->getImpure(impureOffset); ExtEngineManager::ResultSet*& resultSet = request->req_ext_resultset; @@ -462,27 +470,27 @@ namespace { } - string internalPrint(NodePrinter& printer) const + string internalPrint(NodePrinter& printer) const override { StmtNode::internalPrint(printer); return "ExtTriggerNode"; } - void genBlr(DsqlCompilerScratch* /*dsqlScratch*/) + void genBlr(DsqlCompilerScratch* /*dsqlScratch*/) override { } - ExtTriggerNode* pass1(thread_db* /*tdbb*/, CompilerScratch* /*csb*/) + ExtTriggerNode* pass1(thread_db* /*tdbb*/, CompilerScratch* /*csb*/) override { return this; } - ExtTriggerNode* pass2(thread_db* /*tdbb*/, CompilerScratch* /*csb*/) + ExtTriggerNode* pass2(thread_db* /*tdbb*/, CompilerScratch* /*csb*/) override { return this; } - const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* /*exeState*/) const + const StmtNode* execute(thread_db* tdbb, Request* request, ExeState* /*exeState*/) const override { if (request->req_operation == Request::req_evaluate) { @@ -502,7 +510,6 @@ namespace &request->req_rpb[n] : NULL; } - private: const ExtEngineManager::Trigger* trigger; }; @@ -757,13 +764,133 @@ void ExtEngineManager::ExtRoutine::PluginDeleter::operator()(IPluginBase* ptr) //--------------------- -ExtEngineManager::Function::Function(thread_db* tdbb, ExtEngineManager* aExtManager, - IExternalEngine* aEngine, RoutineMetadata* aMetadata, IExternalFunction* aFunction, +struct ExtEngineManager::Function::Impl final +{ + Impl(MemoryPool& pool) + : inValidations(pool), + outValidations(pool), + outDefaults(pool) + { + } + + Array> inValidations; + Array> outValidations; + Array outDefaults; +}; + + +ExtEngineManager::Function::Function(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, + ExtEngineManager* aExtManager, IExternalEngine* aEngine, + RoutineMetadata* aMetadata, IExternalFunction* aFunction, + RefPtr extInputParameters, RefPtr extOutputParameters, const Jrd::Function* aUdf) : ExtRoutine(tdbb, aExtManager, aEngine, aMetadata), function(aFunction), - udf(aUdf) + udf(aUdf), + impl(FB_NEW_POOL(pool) Impl(pool)) { + extInputFormat.reset(Routine::createFormat(pool, extInputParameters, false)); + extOutputFormat.reset(Routine::createFormat(pool, extOutputParameters, true)); + + const bool useExtInMessage = udf->getInputFields().hasData() && + !sameFormats(extInputFormat, udf->getInputFormat()); + + if (useExtInMessage) + extInputImpureOffset = csb->allocImpure(FB_ALIGNMENT, extInputFormat->fmt_length); + else + extInputFormat.reset(); + + const bool useExtOutMessage = udf->getOutputFields().hasData() && + !sameFormats(extOutputFormat, udf->getOutputFormat()); + + if (useExtOutMessage) + extOutputImpureOffset = csb->allocImpure(FB_ALIGNMENT, extOutputFormat->fmt_length); + else + extOutputFormat.reset(); + + // Get input parameters that have validation expressions. + for (const auto param : udf->getInputFields()) + { + FieldInfo fieldInfo; + ItemInfo itemInfo; + + if (param->prm_mechanism != prm_mech_type_of && + !fb_utils::implicit_domain(param->prm_field_source.c_str())) + { + const MetaNamePair namePair(param->prm_field_source, ""); + const bool exist = csb->csb_map_field_info.get(namePair, fieldInfo); + + if (!exist) + { + dsc dummyDesc; + MET_get_domain(tdbb, csb->csb_pool, param->prm_field_source, &dummyDesc, &fieldInfo); + csb->csb_map_field_info.put(namePair, fieldInfo); + } + + itemInfo.field = namePair; + itemInfo.nullable = fieldInfo.nullable; + itemInfo.fullDomain = true; + } + + itemInfo.name = param->prm_name; + + if (!param->prm_nullable) + itemInfo.nullable = false; + + if (itemInfo.isSpecial()) + { + Item item(Item::TYPE_PARAMETER, 0, (param->prm_number - 1) * 2); + csb->csb_map_item_info.put(item, itemInfo); + + impl->inValidations.ensureCapacity(udf->getInputFields().getCount() - (param->prm_number - 1)); + impl->inValidations.push({item, CMP_pass2_validation(tdbb, csb, item)}); + } + } + + // Get output parameters that have default or validation expressions. + for (const auto param : udf->getOutputFields()) + { + FieldInfo fieldInfo; + ItemInfo itemInfo; + + if (param->prm_mechanism != prm_mech_type_of && + !fb_utils::implicit_domain(param->prm_field_source.c_str())) + { + const MetaNamePair namePair(param->prm_field_source, ""); + const bool exist = csb->csb_map_field_info.get(namePair, fieldInfo); + + if (!exist) + { + dsc dummyDesc; + MET_get_domain(tdbb, csb->csb_pool, param->prm_field_source, &dummyDesc, &fieldInfo); + csb->csb_map_field_info.put(namePair, fieldInfo); + } + + if (fieldInfo.defaultValue) + { + impl->outDefaults.ensureCapacity(udf->getOutputFields().getCount() - param->prm_number); + impl->outDefaults.push(param->prm_number); + } + + itemInfo.field = namePair; + itemInfo.nullable = fieldInfo.nullable; + itemInfo.fullDomain = true; + } + + itemInfo.name = param->prm_name; + + if (!param->prm_nullable) + itemInfo.nullable = false; + + if (itemInfo.isSpecial()) + { + Item item(Item::TYPE_PARAMETER, 1, param->prm_number * 2); + csb->csb_map_item_info.put(item, itemInfo); + + impl->outValidations.ensureCapacity(udf->getOutputFields().getCount()); + impl->outValidations.push({item, CMP_pass2_validation(tdbb, csb, item)}); + } + } } @@ -774,20 +901,124 @@ ExtEngineManager::Function::~Function() } -void ExtEngineManager::Function::execute(thread_db* tdbb, UCHAR* inMsg, UCHAR* outMsg) const +// Execute an external function starting with an inactive request and ending with an active one. +void ExtEngineManager::Function::execute(thread_db* tdbb, Request* request, jrd_tra* transaction, + unsigned inMsgLength, UCHAR* inMsg, unsigned outMsgLength, UCHAR* outMsg) const { - EngineAttachmentInfo* attInfo = extManager->getEngineAttachment(tdbb, engine.get()); - const MetaString& userName = udf->invoker ? udf->invoker->getUserName() : ""; - ContextManager ctxManager(tdbb, attInfo, function, - (udf->getName().package.isEmpty() ? - CallerName(obj_udf, udf->getName().identifier, userName) : - CallerName(obj_package_header, udf->getName().package, userName))); + AutoSetRestore2 autoSetRequest( + tdbb, &thread_db::getRequest, &thread_db::setRequest, request); - EngineCheckout cout(tdbb, FB_FUNCTION, checkoutType(attInfo->engine)); + EXE_activate(tdbb, request, transaction); - FbLocalStatus status; - function->execute(&status, attInfo->context, inMsg, outMsg); - status.check(); + fb_assert(inMsgLength == udf->getInputFormat()->fmt_length); + fb_assert(outMsgLength == udf->getOutputFormat()->fmt_length); + + // Validate input parameters (internal message). + validateParameters(tdbb, inMsg, true); + + // If there is a need for an external input message, copy the internal message to it and switch the message. + if (extInputImpureOffset.has_value()) + { + const auto extInMsg = request->getImpure(extInputImpureOffset.value()); + copyMessage(tdbb, udf->getInputFormat(), inMsg, extInputFormat, extInMsg); + inMsg = extInMsg; + ///inMsgLength = extInputFormat->fmt_length; + } + + // Initialize outputs in the internal message. + { // scope + fb_assert(udf->getOutputFormat()->fmt_desc.getCount() / 2 == udf->getOutputFields().getCount()); + + // Initialize everything to NULL (-1). + memset(outMsg, 0xFF, udf->getOutputFormat()->fmt_length); + + for (const auto paramNumber : impl->outDefaults) + { + const auto param = udf->getOutputFields()[paramNumber]; + const MetaNamePair namePair(param->prm_field_source, ""); + FieldInfo fieldInfo; + + dsc* defaultValue = nullptr; + + if (request->getStatement()->mapFieldInfo.get(namePair, fieldInfo) && + fieldInfo.defaultValue) + { + defaultValue = EVL_expr(tdbb, request, fieldInfo.defaultValue); + + if (request->req_flags & req_null) + defaultValue = nullptr; + } + + const auto& paramDesc = udf->getOutputFormat()->fmt_desc[paramNumber * 2]; + const auto& nullDesc = udf->getOutputFormat()->fmt_desc[paramNumber * 2 + 1]; + + fb_assert(nullDesc.dsc_dtype == dtype_short); + + if (defaultValue) + { + dsc desc = paramDesc; + desc.dsc_address = outMsg + (IPTR) desc.dsc_address; + MOV_move(tdbb, defaultValue, &desc); + + *(SSHORT*) (outMsg + (IPTR) nullDesc.dsc_address) = 0; + } + else + *(SSHORT*) (outMsg + (IPTR) nullDesc.dsc_address) = -1; + } + } + + const auto extOutMsg = extOutputImpureOffset.has_value() ? + request->getImpure(extOutputImpureOffset.value()) : nullptr; + + // If there is a need for an external output message, copy the internal message to it. + if (extOutMsg) + copyMessage(tdbb, udf->getOutputFormat(), outMsg, extOutputFormat, extOutMsg); + + // Call external. + { // scope + EngineAttachmentInfo* attInfo = extManager->getEngineAttachment(tdbb, engine.get()); + const MetaString& userName = udf->invoker ? udf->invoker->getUserName() : ""; + ContextManager ctxManager(tdbb, attInfo, function, + (udf->getName().package.isEmpty() ? + CallerName(obj_udf, udf->getName().identifier, userName) : + CallerName(obj_package_header, udf->getName().package, userName))); + + EngineCheckout cout(tdbb, FB_FUNCTION, checkoutType(attInfo->engine)); + + FbLocalStatus status; + function->execute(&status, attInfo->context, inMsg, (extOutMsg ? extOutMsg : outMsg)); + status.check(); + } + + // If there is an external output message, copy it to the internal message. + if (extOutMsg) + copyMessage(tdbb, extOutputFormat, extOutMsg, udf->getOutputFormat(), outMsg); + + // Validate output parameters (internal message). + validateParameters(tdbb, outMsg, false); +} + +void ExtEngineManager::Function::validateParameters(thread_db* tdbb, UCHAR* msg, bool input) const +{ + const auto format = input ? udf->getInputFormat() : udf->getOutputFormat(); + const auto& validations = input ? impl->inValidations : impl->outValidations; + const UCHAR messageNumber = input ? 0 : 1; + + for (const auto& [item, itemInfo] : validations) + { + const unsigned paramNumber = item.index / 2; + const auto& paramDesc = format->fmt_desc[paramNumber * 2]; + const auto& nullDesc = format->fmt_desc[paramNumber * 2 + 1]; + + fb_assert(nullDesc.dsc_dtype == dtype_short); + + dsc value = paramDesc; + value.dsc_address = msg + (IPTR) value.dsc_address; + + const auto isNull = *(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) != 0; + + EVL_validate(tdbb, Item(Item::TYPE_PARAMETER, messageNumber, paramNumber), itemInfo, &value, isNull); + } } @@ -1391,63 +1622,16 @@ void ExtEngineManager::makeFunction(thread_db* tdbb, CompilerScratch* csb, Jrd:: status.check(); } - const Format* extInputFormat = Routine::createFormat(pool, extInputParameters, false); - const Format* extOutputFormat = Routine::createFormat(pool, extOutputParameters, true); - try { - udf->fun_external = FB_NEW_POOL(pool) Function(tdbb, this, attInfo->engine, - metadata.release(), externalFunction, udf); - - MemoryPool& csbPool = csb->csb_pool; - - CompoundStmtNode* mainNode = FB_NEW_POOL(csbPool) CompoundStmtNode(csbPool); - - IntMessageNode* intInMessageNode = udf->getInputFields().hasData() ? - FB_NEW_POOL(csbPool) IntMessageNode(tdbb, csbPool, csb, 0, - udf->getInputFields(), udf->getInputFormat()) : - NULL; - ExtMessageNode* extInMessageNode = NULL; - - if (intInMessageNode) - { - mainNode->statements.add(intInMessageNode); + udf->fun_external = FB_NEW_POOL(pool) Function(tdbb, pool, csb, this, attInfo->engine, + metadata.release(), externalFunction, extInputParameters, extOutputParameters, udf); - extInMessageNode = FB_NEW_POOL(csbPool) ExtMessageNode(tdbb, csbPool, csb, 2, extInputFormat); - mainNode->statements.add(extInMessageNode); - } - - IntMessageNode* intOutMessageNode = FB_NEW_POOL(csbPool) IntMessageNode(tdbb, csbPool, csb, 1, - udf->getOutputFields(), udf->getOutputFormat()); - mainNode->statements.add(intOutMessageNode); + // This is necessary for compilation, but will never be executed. + const auto dummyNode = FB_NEW_POOL(csb->csb_pool) CompoundStmtNode(csb->csb_pool); - ExtMessageNode* extOutMessageNode = FB_NEW_POOL(csbPool) ExtMessageNode(tdbb, csbPool, csb, 3, - extOutputFormat); - mainNode->statements.add(extOutMessageNode); - - // Initialize the output fields into the external message. - InitOutputNode* initOutputNode = FB_NEW_POOL(csbPool) InitOutputNode( - tdbb, csbPool, csb, udf->getOutputFields(), extOutMessageNode); - mainNode->statements.add(initOutputNode); - - if (intInMessageNode) - { - ReceiveNode* receiveNode = intInMessageNode ? FB_NEW_POOL(csbPool) ReceiveNode(csbPool) : NULL; - receiveNode->message = intInMessageNode; - receiveNode->statement = FB_NEW_POOL(csbPool) MessageMoverNode( - csbPool, intInMessageNode, extInMessageNode); - mainNode->statements.add(receiveNode); - } - - ExtFunctionNode* extFunctionNode = FB_NEW_POOL(csbPool) ExtFunctionNode(csbPool, - extInMessageNode, extOutMessageNode, udf->fun_external); - mainNode->statements.add(extFunctionNode); - extFunctionNode->message = intOutMessageNode; - extFunctionNode->statement = FB_NEW_POOL(csbPool) MessageMoverNode( - csbPool, extOutMessageNode, intOutMessageNode); - - Statement* statement = udf->getStatement(); - PAR_preparsed_node(tdbb, NULL, mainNode, NULL, &csb, &statement, false, 0); + auto statement = udf->getStatement(); + PAR_preparsed_node(tdbb, nullptr, dummyNode, nullptr, &csb, &statement, false, 0); udf->setStatement(statement); } catch (...) @@ -1563,9 +1747,9 @@ void ExtEngineManager::makeProcedure(thread_db* tdbb, CompilerScratch* csb, jrd_ mainNode->statements.add(extOutMessageNode); // Initialize the output fields into the external message. - InitOutputNode* initOutputNode = FB_NEW_POOL(csbPool) InitOutputNode( + InitParametersNode* initParametersNode = FB_NEW_POOL(csbPool) InitParametersNode( tdbb, csbPool, csb, prc->getOutputFields(), extOutMessageNode); - mainNode->statements.add(initOutputNode); + mainNode->statements.add(initParametersNode); ReceiveNode* receiveNode = intInMessageNode ? FB_NEW_POOL(csbPool) ReceiveNode(csbPool) : NULL; diff --git a/src/jrd/ExtEngineManager.h b/src/jrd/ExtEngineManager.h index 24106148ea7..c75db6a6fcd 100644 --- a/src/jrd/ExtEngineManager.h +++ b/src/jrd/ExtEngineManager.h @@ -26,6 +26,7 @@ #include "firebird/Interface.h" #include +#include #include "../common/classes/array.h" #include "../common/classes/fb_string.h" @@ -218,6 +219,7 @@ class ExtEngineManager final : public Firebird::PermanentStorage public: ExtRoutine(thread_db* tdbb, ExtEngineManager* aExtManager, Firebird::IExternalEngine* aEngine, RoutineMetadata* aMetadata); + virtual ~ExtRoutine() = default; private: class PluginDeleter @@ -233,42 +235,56 @@ class ExtEngineManager final : public Firebird::PermanentStorage Database* database; }; - class Function : public ExtRoutine + class Function final : public ExtRoutine { + private: + struct Impl; // hack to avoid circular inclusion of headers + public: - Function(thread_db* tdbb, ExtEngineManager* aExtManager, + Function(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, ExtEngineManager* aExtManager, Firebird::IExternalEngine* aEngine, RoutineMetadata* aMetadata, Firebird::IExternalFunction* aFunction, + Firebird::RefPtr extInputParameters, + Firebird::RefPtr extOutputParameters, const Jrd::Function* aUdf); - ~Function(); + ~Function() override; - void execute(thread_db* tdbb, UCHAR* inMsg, UCHAR* outMsg) const; + void execute(thread_db* tdbb, Request* request, jrd_tra* transaction, + unsigned inMsgLength, UCHAR* inMsg, unsigned outMsgLength, UCHAR* outMsg) const; + + private: + void validateParameters(thread_db* tdbb, UCHAR* msg, bool input) const; private: Firebird::IExternalFunction* function; const Jrd::Function* udf; + Firebird::AutoPtr extInputFormat; + Firebird::AutoPtr extOutputFormat; + Firebird::AutoPtr impl; + std::optional extInputImpureOffset; + std::optional extOutputImpureOffset; }; class ResultSet; - class Procedure : public ExtRoutine + class Procedure final : public ExtRoutine { + friend class ResultSet; + public: Procedure(thread_db* tdbb, ExtEngineManager* aExtManager, Firebird::IExternalEngine* aEngine, RoutineMetadata* aMetadata, Firebird::IExternalProcedure* aProcedure, const jrd_prc* aPrc); - ~Procedure(); + ~Procedure() override; ResultSet* open(thread_db* tdbb, UCHAR* inMsg, UCHAR* outMsg) const; private: Firebird::IExternalProcedure* procedure; const jrd_prc* prc; - - friend class ResultSet; }; class ResultSet @@ -288,13 +304,13 @@ class ExtEngineManager final : public Firebird::PermanentStorage USHORT charSet; }; - class Trigger : public ExtRoutine + class Trigger final : public ExtRoutine { public: Trigger(thread_db* tdbb, MemoryPool& pool, CompilerScratch* csb, ExtEngineManager* aExtManager, Firebird::IExternalEngine* aEngine, RoutineMetadata* aMetadata, Firebird::IExternalTrigger* aTrigger, const Jrd::Trigger* aTrg); - ~Trigger(); + ~Trigger() override; void execute(thread_db* tdbb, Request* request, unsigned action, record_param* oldRpb, record_param* newRpb) const; diff --git a/src/jrd/Function.h b/src/jrd/Function.h index 5ddb9acd40d..289a28fc9b0 100644 --- a/src/jrd/Function.h +++ b/src/jrd/Function.h @@ -32,7 +32,7 @@ namespace Jrd class ValueListNode; class QualifiedName; - class Function : public Routine + class Function final : public Routine { static const char* const EXCEPTION_MESSAGE; @@ -58,29 +58,30 @@ namespace Jrd static int blockingAst(void*); public: - virtual int getObjectType() const + int getObjectType() const override { return obj_udf; } - virtual SLONG getSclType() const + SLONG getSclType() const override { return obj_functions; } - virtual bool checkCache(thread_db* tdbb) const; - virtual void clearCache(thread_db* tdbb); + bool checkCache(thread_db* tdbb) const override; + void clearCache(thread_db* tdbb) override; - virtual ~Function() + ~Function() override { delete fun_external; } - virtual void releaseExternal() + void releaseExternal() override { delete fun_external; fun_external = NULL; } + public: int (*fun_entrypoint)(); // function entrypoint USHORT fun_inputs; // input arguments @@ -93,7 +94,7 @@ namespace Jrd const ExtEngineManager::Function* fun_external; protected: - virtual bool reload(thread_db* tdbb); + bool reload(thread_db* tdbb) override; }; } diff --git a/src/jrd/exe.cpp b/src/jrd/exe.cpp index e9f348b1678..e2bc6586392 100644 --- a/src/jrd/exe.cpp +++ b/src/jrd/exe.cpp @@ -848,18 +848,9 @@ void EXE_send(thread_db* tdbb, Request* request, USHORT msg, ULONG length, const } -void EXE_start(thread_db* tdbb, Request* request, jrd_tra* transaction) +// Mark a request as active. +void EXE_activate(thread_db* tdbb, Request* request, jrd_tra* transaction) { -/************************************** - * - * E X E _ s t a r t - * - ************************************** - * - * Functional description - * Start an execution running. - * - **************************************/ SET_TDBB(tdbb); BLKCHK(request, type_req); @@ -923,10 +914,15 @@ void EXE_start(thread_db* tdbb, Request* request, jrd_tra* transaction) request->req_src_column = 0; TRA_setup_request_snapshot(tdbb, request); +} + + +// Start and execute a request. +void EXE_start(thread_db* tdbb, Request* request, jrd_tra* transaction) +{ + EXE_activate(tdbb, request, transaction); - execute_looper(tdbb, request, transaction, - request->getStatement()->topNode, - Request::req_evaluate); + execute_looper(tdbb, request, transaction, request->getStatement()->topNode, Request::req_evaluate); } diff --git a/src/jrd/exe.h b/src/jrd/exe.h index 4b945f6fa7e..3e348217024 100644 --- a/src/jrd/exe.h +++ b/src/jrd/exe.h @@ -428,9 +428,8 @@ class ItemInfo : public Printable bool fullDomain; }; -typedef Firebird::GenericMap > > - MapFieldInfo; -typedef Firebird::GenericMap > > MapItemInfo; +typedef Firebird::LeftPooledMap MapFieldInfo; +typedef Firebird::RightPooledMap MapItemInfo; // Compile scratch block diff --git a/src/jrd/exe_proto.h b/src/jrd/exe_proto.h index 39365aec0e2..3f548e230ad 100644 --- a/src/jrd/exe_proto.h +++ b/src/jrd/exe_proto.h @@ -51,6 +51,7 @@ void EXE_execute_triggers(Jrd::thread_db*, Jrd::TrigVector**, Jrd::record_param* void EXE_receive(Jrd::thread_db*, Jrd::Request*, USHORT, ULONG, void*, bool = false); void EXE_release(Jrd::thread_db*, Jrd::Request*); void EXE_send(Jrd::thread_db*, Jrd::Request*, USHORT, ULONG, const void*); +void EXE_activate(Jrd::thread_db*, Jrd::Request*, Jrd::jrd_tra*); void EXE_start(Jrd::thread_db*, Jrd::Request*, Jrd::jrd_tra*); void EXE_unwind(Jrd::thread_db*, Jrd::Request*); diff --git a/src/jrd/jrd.h b/src/jrd/jrd.h index f5eb377619d..a4e5257948b 100644 --- a/src/jrd/jrd.h +++ b/src/jrd/jrd.h @@ -211,7 +211,7 @@ const int DYN_REQUESTS = 2; // Procedure block -class jrd_prc : public Routine +class jrd_prc final : public Routine { public: const Format* prc_record_format; @@ -233,38 +233,38 @@ class jrd_prc : public Routine } public: - virtual int getObjectType() const + int getObjectType() const override { return obj_procedure; } - virtual SLONG getSclType() const + SLONG getSclType() const override { return obj_procedures; } - virtual void releaseFormat() + void releaseFormat() override { delete prc_record_format; prc_record_format = NULL; } - virtual ~jrd_prc() + ~jrd_prc() override { delete prc_external; } - virtual bool checkCache(thread_db* tdbb) const; - virtual void clearCache(thread_db* tdbb); + bool checkCache(thread_db* tdbb) const override; + void clearCache(thread_db* tdbb) override; - virtual void releaseExternal() + void releaseExternal() override { delete prc_external; prc_external = NULL; } protected: - virtual bool reload(thread_db* tdbb); // impl is in met.epp + bool reload(thread_db* tdbb) override; // impl is in met.epp }; From 178bb0f8d7fd41dec5bb8e330e1fbbb727292608 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Wed, 28 Feb 2024 07:35:09 -0300 Subject: [PATCH 2/2] Changes after Dmitry review. --- src/jrd/ExtEngineManager.cpp | 43 +++++++++++++----------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/src/jrd/ExtEngineManager.cpp b/src/jrd/ExtEngineManager.cpp index b5319e214ca..4726985f64f 100644 --- a/src/jrd/ExtEngineManager.cpp +++ b/src/jrd/ExtEngineManager.cpp @@ -109,11 +109,9 @@ namespace dstDesc.dsc_address = dstMsg + dstArgOffset; MOV_move(tdbb, &srcDesc, &dstDesc); - - *dstNullPtr = 0; } - else - *dstNullPtr = -1; + + *dstNullPtr = *srcNullPtr; srcDescIt += 2; dstDescIt += 2; @@ -280,13 +278,8 @@ namespace dsc* defaultDesc = nullptr; if (defaultValueNode) - { defaultDesc = EVL_expr(tdbb, request, defaultValueNode); - if (request->req_flags & req_null) - defaultDesc = nullptr; - } - const auto formatIndex = paramIndex * 2; const auto& nullDesc = message->format->fmt_desc[formatIndex + 1]; fb_assert(nullDesc.dsc_dtype == dtype_short); @@ -294,14 +287,14 @@ namespace if (defaultDesc) { // Initialize the value. The null flag is already initialized to not-null. - fb_assert(*(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) != -1); + fb_assert(!*(SSHORT*) (msg + (IPTR) nullDesc.dsc_address)); dsc desc = message->format->fmt_desc[formatIndex]; desc.dsc_address = msg + (IPTR) desc.dsc_address; MOV_move(tdbb, defaultDesc, &desc); } else - *(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) = -1; + *(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) = FB_TRUE; } request->req_operation = Request::req_return; @@ -926,11 +919,11 @@ void ExtEngineManager::Function::execute(thread_db* tdbb, Request* request, jrd_ } // Initialize outputs in the internal message. - { // scope + { fb_assert(udf->getOutputFormat()->fmt_desc.getCount() / 2 == udf->getOutputFields().getCount()); - // Initialize everything to NULL (-1). - memset(outMsg, 0xFF, udf->getOutputFormat()->fmt_length); + // Initialize everything to NULL (FB_TRUE). + memset(outMsg, FB_TRUE, udf->getOutputFormat()->fmt_length); for (const auto paramNumber : impl->outDefaults) { @@ -940,15 +933,9 @@ void ExtEngineManager::Function::execute(thread_db* tdbb, Request* request, jrd_ dsc* defaultValue = nullptr; - if (request->getStatement()->mapFieldInfo.get(namePair, fieldInfo) && - fieldInfo.defaultValue) - { + if (request->getStatement()->mapFieldInfo.get(namePair, fieldInfo) && fieldInfo.defaultValue) defaultValue = EVL_expr(tdbb, request, fieldInfo.defaultValue); - if (request->req_flags & req_null) - defaultValue = nullptr; - } - const auto& paramDesc = udf->getOutputFormat()->fmt_desc[paramNumber * 2]; const auto& nullDesc = udf->getOutputFormat()->fmt_desc[paramNumber * 2 + 1]; @@ -960,10 +947,10 @@ void ExtEngineManager::Function::execute(thread_db* tdbb, Request* request, jrd_ desc.dsc_address = outMsg + (IPTR) desc.dsc_address; MOV_move(tdbb, defaultValue, &desc); - *(SSHORT*) (outMsg + (IPTR) nullDesc.dsc_address) = 0; + *(SSHORT*) (outMsg + (IPTR) nullDesc.dsc_address) = FB_FALSE; } else - *(SSHORT*) (outMsg + (IPTR) nullDesc.dsc_address) = -1; + *(SSHORT*) (outMsg + (IPTR) nullDesc.dsc_address) = FB_TRUE; } } @@ -1015,7 +1002,7 @@ void ExtEngineManager::Function::validateParameters(thread_db* tdbb, UCHAR* msg, dsc value = paramDesc; value.dsc_address = msg + (IPTR) value.dsc_address; - const auto isNull = *(SSHORT*) (msg + (IPTR) nullDesc.dsc_address) != 0; + const bool isNull = *(SSHORT*) (msg + (IPTR) nullDesc.dsc_address); EVL_validate(tdbb, Item(Item::TYPE_PARAMETER, messageNumber, paramNumber), itemInfo, &value, isNull); } @@ -1339,9 +1326,9 @@ void ExtEngineManager::Trigger::setValues(thread_db* tdbb, Request* request, Arr const DeclareVariableNode* varDecl = varDecls[computedVarId++]; impure_value* varImpure = request->getImpure(varDecl->impureOffset); - *nullTarget = (varImpure->vlu_desc.dsc_flags & DSC_null) != 0 ? -1 : 0; + *nullTarget = (varImpure->vlu_desc.dsc_flags & DSC_null) ? FB_TRUE : FB_FALSE; - if (*nullTarget == 0) + if (!*nullTarget) MOV_move(tdbb, &varImpure->vlu_desc, &target); } else @@ -1349,9 +1336,9 @@ void ExtEngineManager::Trigger::setValues(thread_db* tdbb, Request* request, Arr if (!EVL_field(rpb->rpb_relation, rpb->rpb_record, fieldPos, &source)) source.dsc_flags |= DSC_null; - *nullTarget = (source.dsc_flags & DSC_null) != 0 ? -1 : 0; + *nullTarget = (source.dsc_flags & DSC_null) ? FB_TRUE : FB_FALSE; - if (*nullTarget == 0) + if (!*nullTarget) MOV_move(tdbb, &source, &target); } }