From 07c8e84a3a133ed3fda602c8e4b8492515ebef84 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Fri, 21 Jun 2024 20:56:35 -0400 Subject: [PATCH] GH-423 Avoid reuse of default value instances --- src/script/instances/node_instance.cpp | 25 ++++++++++++++++++ src/script/instances/node_instance.h | 4 +++ src/script/vm/execution_context.h | 7 +++++- src/script/vm/script_vm.cpp | 35 ++++++++++++++++++++------ src/script/vm/script_vm.h | 3 ++- 5 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/script/instances/node_instance.cpp b/src/script/instances/node_instance.cpp index 06abab4d..2b1b755b 100644 --- a/src/script/instances/node_instance.cpp +++ b/src/script/instances/node_instance.cpp @@ -27,3 +27,28 @@ Ref OScriptNodeInstance::get_base_node() { return { _base }; } + +OScriptNodeInstance::~OScriptNodeInstance() +{ + if (input_pin_count > 0) + { + if (input_pins != nullptr) + memdelete_arr(input_pins); + if (input_default_stack_pos != nullptr) + memdelete_arr(input_default_stack_pos); + } + + if (output_pin_count > 0) + { + if (output_pins != nullptr) + memdelete_arr(output_pins); + } + + if (execution_output_pin_count > 0) + { + if (execution_output_pins != nullptr) + memdelete_arr(execution_output_pins); + if (execution_outputs != nullptr) + memdelete_arr(execution_outputs); + } +} diff --git a/src/script/instances/node_instance.h b/src/script/instances/node_instance.h index ebfef56a..e145e465 100644 --- a/src/script/instances/node_instance.h +++ b/src/script/instances/node_instance.h @@ -86,6 +86,7 @@ class OScriptNodeInstance : public Object Vector dependencies; //! List of node instance dependencies for this node int* input_pins{ nullptr }; //! Input pins int input_pin_count{ 0 }; //! Input pin count + int* input_default_stack_pos{ nullptr }; //! Holds stack position references for default values int* output_pins{ nullptr }; //! Output pins int output_pin_count{ 0 }; //! Output pin count int working_memory_index{ 0 }; //! Number of working memory slots @@ -109,6 +110,9 @@ class OScriptNodeInstance : public Object /// @param p_context execution context /// @return the output port and bits virtual int step(OScriptExecutionContext& p_context) = 0; + + /// Destructor + ~OScriptNodeInstance() override; }; #endif // ORCHESTRATOR_SCRIPT_NODE_INSTANCE_H \ No newline at end of file diff --git a/src/script/vm/execution_context.h b/src/script/vm/execution_context.h index 55ed0066..86906c66 100644 --- a/src/script/vm/execution_context.h +++ b/src/script/vm/execution_context.h @@ -113,8 +113,13 @@ class OScriptExecutionContext /// Set the input value at the specified index to the default value /// @param p_index the input stack index to mutate + /// @param p_stack_index the stack position to write the value /// @param p_value the default value to use - _FORCE_INLINE_ void _set_input_from_default_value(int p_index, const Variant& p_value) { _inputs[p_index] = const_cast(&p_value); } + _FORCE_INLINE_ void _set_input_from_default_value(int p_index, int p_stack_index, const Variant& p_value) + { + _variant_stack[p_stack_index] = p_value.duplicate(); + _inputs[p_index] = &_variant_stack[p_stack_index]; + } /// Copies a variant stack value to the input stack at the given indices /// @param p_stack_index the variant stack index to copy from diff --git a/src/script/vm/script_vm.cpp b/src/script/vm/script_vm.cpp index 10e97ef6..83214143 100644 --- a/src/script/vm/script_vm.cpp +++ b/src/script/vm/script_vm.cpp @@ -75,20 +75,35 @@ static Ref get_data_pin_at_count_index(const Ref& p return nullptr; } -void OScriptVirtualMachine::_set_unassigned_inputs(const Ref& p_node, const OScriptNodeInstance* p_instance) +void OScriptVirtualMachine::_set_unassigned_inputs(const Ref& p_node, const OScriptNodeInstance* p_instance, Function& r_function) { for (int i = 0; i < p_instance->data_input_pin_count; i++) { // If the input pin is bound with a value of -1, it is unassigned. if (p_instance->input_pins[i] == -1) { - // Assign the default value index - p_instance->input_pins[i] = _default_values.size() | OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT; - // Place the pin's effective default value on the default value list const Ref pin = get_data_pin_at_count_index(p_node, i, PD_Input); - if (pin.is_valid()) + if (!pin.is_valid()) + continue; + + // Default values should be passed on the stack, assign a stack position for the value + int stack_pos = r_function.max_stack++; + p_instance->input_default_stack_pos[i] = stack_pos; + + // Rather than duplicate default values for each node, reuse existing ones if possible. + int lookup_index = _default_values.find(pin->get_effective_default_value()); + if (lookup_index != -1) + { + // Default value already exists, assign the stack position + p_instance->input_pins[i] = lookup_index | OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT; + } + else + { + // Default value doesn't exist, create a new entry + p_instance->input_pins[i] = _default_values.size() | OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT; _default_values.push_back(pin->get_effective_default_value()); + } } } } @@ -198,8 +213,12 @@ bool OScriptVirtualMachine::_create_node_instance_pins(const Ref& p // Create the input array // Each defaults to -1, and if left as -1 triggers default value serialization p_instance->input_pins = memnew_arr(int, p_instance->data_input_pin_count); + p_instance->input_default_stack_pos = memnew_arr(int, p_instance->data_input_pin_count); for (int i = 0; i < p_instance->data_input_pin_count; i++) + { p_instance->input_pins[i] = -1; + p_instance->input_default_stack_pos[i] = -1; + } } if (p_instance->data_output_pin_count) @@ -407,7 +426,7 @@ bool OScriptVirtualMachine::_build_function_node_graph(const Ref& node = p_function->get_orchestration()->get_node(E); const OScriptNodeInstance* instance = _nodes[E]; - _set_unassigned_inputs(node, instance); + _set_unassigned_inputs(node, instance, r_function); _set_unassigned_outputs(node, instance, r_function.trash_pos); } @@ -448,7 +467,7 @@ void OScriptVirtualMachine::_resolve_inputs(OScriptExecutionContext& p_context, { const int index = p_instance->input_pins[i] & OScriptNodeInstance::INPUT_MASK; if (p_instance->input_pins[i] & OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT) - p_context._set_input_from_default_value(i, _default_values[index]); + p_context._set_input_from_default_value(i, p_instance->input_default_stack_pos[i], _default_values[index]); else p_context._copy_stack_to_input(index, i); } @@ -509,7 +528,7 @@ void OScriptVirtualMachine::_dependency_step(OScriptExecutionContext& p_context, { const int index = p_instance->input_pins[i] & OScriptNodeInstance::INPUT_MASK; if (p_instance->input_pins[i] & OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT) - p_context.set_input(i, &_default_values[index]); + p_context._set_input_from_default_value(i, p_instance->input_default_stack_pos[i], _default_values[index]); else p_context._copy_stack_to_input(index, i); } diff --git a/src/script/vm/script_vm.h b/src/script/vm/script_vm.h index 4e37acd9..66ae67c8 100644 --- a/src/script/vm/script_vm.h +++ b/src/script/vm/script_vm.h @@ -80,7 +80,8 @@ class OScriptVirtualMachine /// Sets unassigned inputs on the specified node, if any exist. /// @param p_node the script node /// @param p_instance the node instance - void _set_unassigned_inputs(const Ref& p_node, const OScriptNodeInstance* p_instance); + /// @param r_function the function declaration + void _set_unassigned_inputs(const Ref& p_node, const OScriptNodeInstance* p_instance, Function& r_function); /// Sets unassigned outputs on the specified node, if any exist. /// @param p_node the script node