Skip to content

Commit

Permalink
Merge pull request #86860 from rune-scape/rune-fix-lambda-hotswap2-4.2
Browse files Browse the repository at this point in the history 
[4.2] GDScript: Lambda hotswap fixes
  • Loading branch information
@akien-mga
akien-mga authored Jan 18, 2024
2 parents 24c0d26 + 4d4ec47 commit 1cb7d6f
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 176 deletions.
129 changes: 38 additions & 91 deletions modules/gdscript/gdscript.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1391,51 +1391,43 @@ String GDScript::debug_get_script_name(const Ref<Script> &p_script) {
}
#endif

GDScript::UpdatableFuncPtr GDScript::func_ptrs_to_update_main_thread;
thread_local GDScript::UpdatableFuncPtr *GDScript::func_ptrs_to_update_thread_local = nullptr;

GDScript::UpdatableFuncPtrElement GDScript::_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr) {
UpdatableFuncPtrElement result = {};

{
MutexLock lock(func_ptrs_to_update_thread_local->mutex);
result.element = func_ptrs_to_update_thread_local->ptrs.push_back(p_func_ptr_ptr);
result.func_ptr = func_ptrs_to_update_thread_local;

if (likely(func_ptrs_to_update_thread_local->initialized)) {
return result;
}

func_ptrs_to_update_thread_local->initialized = true;
GDScript::UpdatableFuncPtr::UpdatableFuncPtr(GDScriptFunction *p_function) {
if (p_function == nullptr) {
return;
}

MutexLock lock(func_ptrs_to_update_mutex);
func_ptrs_to_update.push_back(func_ptrs_to_update_thread_local);
func_ptrs_to_update_thread_local->rc++;

return result;
}
ptr = p_function;
script = ptr->get_script();
ERR_FAIL_NULL(script);

void GDScript::_remove_func_ptr_to_update(const UpdatableFuncPtrElement &p_func_ptr_element) {
ERR_FAIL_NULL(p_func_ptr_element.element);
ERR_FAIL_NULL(p_func_ptr_element.func_ptr);
MutexLock lock(p_func_ptr_element.func_ptr->mutex);
p_func_ptr_element.element->erase();
MutexLock script_lock(script->func_ptrs_to_update_mutex);
list_element = script->func_ptrs_to_update.push_back(this);
}

void GDScript::_fixup_thread_function_bookkeeping() {
// Transfer the ownership of these update items to the main thread,
// because the current one is dying, leaving theirs orphan, dangling.
GDScript::UpdatableFuncPtr::~UpdatableFuncPtr() {
ERR_FAIL_NULL(script);

DEV_ASSERT(!Thread::is_main_thread());
if (list_element) {
MutexLock script_lock(script->func_ptrs_to_update_mutex);
list_element->erase();
list_element = nullptr;
}
}

MutexLock lock(func_ptrs_to_update_main_thread.mutex);
MutexLock lock2(func_ptrs_to_update_thread_local->mutex);
void GDScript::_recurse_replace_function_ptrs(const HashMap<GDScriptFunction *, GDScriptFunction *> &p_replacements) const {
MutexLock lock(func_ptrs_to_update_mutex);
for (UpdatableFuncPtr *updatable : func_ptrs_to_update) {
HashMap<GDScriptFunction *, GDScriptFunction *>::ConstIterator replacement = p_replacements.find(updatable->ptr);
if (replacement) {
updatable->ptr = replacement->value;
} else {
// Probably a lambda from another reload, ignore.
updatable->ptr = nullptr;
}
}

while (!func_ptrs_to_update_thread_local->ptrs.is_empty()) {
List<GDScriptFunction **>::Element *E = func_ptrs_to_update_thread_local->ptrs.front();
E->transfer_to_back(&func_ptrs_to_update_main_thread.ptrs);
func_ptrs_to_update_thread_local->transferred = true;
for (HashMap<StringName, Ref<GDScript>>::ConstIterator subscript = subclasses.begin(); subscript; ++subscript) {
subscript->value->_recurse_replace_function_ptrs(p_replacements);
}
}

Expand All @@ -1457,30 +1449,9 @@ void GDScript::clear(ClearData *p_clear_data) {
}

{
MutexLock outer_lock(func_ptrs_to_update_mutex);
MutexLock lock(func_ptrs_to_update_mutex);
for (UpdatableFuncPtr *updatable : func_ptrs_to_update) {
bool destroy = false;
{
MutexLock inner_lock(updatable->mutex);
if (updatable->transferred) {
func_ptrs_to_update_main_thread.mutex.lock();
}
for (GDScriptFunction **func_ptr_ptr : updatable->ptrs) {
*func_ptr_ptr = nullptr;
}
DEV_ASSERT(updatable->rc != 0);
updatable->rc--;
if (updatable->rc == 0) {
destroy = true;
}
if (updatable->transferred) {
func_ptrs_to_update_main_thread.mutex.unlock();
}
}
if (destroy) {
DEV_ASSERT(updatable != &func_ptrs_to_update_main_thread);
memdelete(updatable);
}
updatable->ptr = nullptr;
}
}

Expand Down Expand Up @@ -1553,6 +1524,13 @@ GDScript::~GDScript() {
}
destructing = true;

if (is_print_verbose_enabled()) {
MutexLock lock(func_ptrs_to_update_mutex);
if (!func_ptrs_to_update.is_empty()) {
print_line(vformat("GDScript: %d orphaned lambdas becoming invalid at destruction of script '%s'.", func_ptrs_to_update.size(), fully_qualified_name));
}
}

clear();

{
Expand Down Expand Up @@ -2101,33 +2079,6 @@ void GDScriptLanguage::remove_named_global_constant(const StringName &p_name) {
named_globals.erase(p_name);
}

void GDScriptLanguage::thread_enter() {
GDScript::func_ptrs_to_update_thread_local = memnew(GDScript::UpdatableFuncPtr);
}

void GDScriptLanguage::thread_exit() {
// This thread may have been created before GDScript was up
// (which also means it can't have run any GDScript code at all).
if (!GDScript::func_ptrs_to_update_thread_local) {
return;
}

GDScript::_fixup_thread_function_bookkeeping();

bool destroy = false;
{
MutexLock lock(GDScript::func_ptrs_to_update_thread_local->mutex);
DEV_ASSERT(GDScript::func_ptrs_to_update_thread_local->rc != 0);
GDScript::func_ptrs_to_update_thread_local->rc--;
if (GDScript::func_ptrs_to_update_thread_local->rc == 0) {
destroy = true;
}
}
if (destroy) {
memdelete(GDScript::func_ptrs_to_update_thread_local);
}
}

void GDScriptLanguage::init() {
//populate global constants
int gcc = CoreConstants::get_global_constant_count();
Expand Down Expand Up @@ -2160,8 +2111,6 @@ void GDScriptLanguage::init() {
_add_global(E.name, E.ptr);
}

GDScript::func_ptrs_to_update_thread_local = &GDScript::func_ptrs_to_update_main_thread;

#ifdef TESTS_ENABLED
GDScriptTests::GDScriptTestRunner::handle_cmdline();
#endif
Expand Down Expand Up @@ -2211,8 +2160,6 @@ void GDScriptLanguage::finish() {
}
script_list.clear();
function_list.clear();

DEV_ASSERT(GDScript::func_ptrs_to_update_main_thread.rc == 1);
}

void GDScriptLanguage::profiling_start() {
Expand Down
42 changes: 18 additions & 24 deletions modules/gdscript/gdscript.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,29 +117,28 @@ class GDScript : public Script {

HashMap<GDScriptFunction *, LambdaInfo> lambda_info;

// List is used here because a ptr to elements are stored, so the memory locations need to be stable
struct UpdatableFuncPtr {
List<GDScriptFunction **> ptrs;
Mutex mutex;
bool initialized : 1;
bool transferred : 1;
uint32_t rc = 1;
UpdatableFuncPtr() :
initialized(false), transferred(false) {}
};
struct UpdatableFuncPtrElement {
List<GDScriptFunction **>::Element *element = nullptr;
UpdatableFuncPtr *func_ptr = nullptr;
public:
class UpdatableFuncPtr {
friend class GDScript;

GDScriptFunction *ptr = nullptr;
GDScript *script = nullptr;
List<UpdatableFuncPtr *>::Element *list_element = nullptr;

public:
GDScriptFunction *operator->() const { return ptr; }
operator GDScriptFunction *() const { return ptr; }

UpdatableFuncPtr(GDScriptFunction *p_function);
~UpdatableFuncPtr();
};
static UpdatableFuncPtr func_ptrs_to_update_main_thread;
static thread_local UpdatableFuncPtr *func_ptrs_to_update_thread_local;

private:
// List is used here because a ptr to elements are stored, so the memory locations need to be stable
List<UpdatableFuncPtr *> func_ptrs_to_update;
Mutex func_ptrs_to_update_mutex;

UpdatableFuncPtrElement _add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr);
static void _remove_func_ptr_to_update(const UpdatableFuncPtrElement &p_func_ptr_element);

static void _fixup_thread_function_bookkeeping();
void _recurse_replace_function_ptrs(const HashMap<GDScriptFunction *, GDScriptFunction *> &p_replacements) const;

#ifdef TOOLS_ENABLED
// For static data storage during hot-reloading.
Expand Down Expand Up @@ -561,11 +560,6 @@ class GDScriptLanguage : public ScriptLanguage {
virtual void add_named_global_constant(const StringName &p_name, const Variant &p_value) override;
virtual void remove_named_global_constant(const StringName &p_name) override;

/* MULTITHREAD FUNCTIONS */

virtual void thread_enter() override;
virtual void thread_exit() override;

/* DEBUGGER FUNCTIONS */

virtual String debug_get_error() const override;
Expand Down
27 changes: 8 additions & 19 deletions modules/gdscript/gdscript_compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
return GDScriptCodeGenerator::Address();
}

main_script->lambda_info.insert(function, { lambda->captures.size(), lambda->use_self });
codegen.script->lambda_info.insert(function, { lambda->captures.size(), lambda->use_self });
gen->write_lambda(result, function, captures, lambda->use_self);

for (int i = 0; i < captures.size(); i++) {
Expand Down Expand Up @@ -3046,7 +3046,7 @@ GDScriptCompiler::FunctionLambdaInfo GDScriptCompiler::_get_function_replacement
FunctionLambdaInfo info;
info.function = p_func;
info.parent = p_parent_func;
info.script = p_parent_func;
info.script = p_func->get_script();
info.name = p_func->get_name();
info.line = p_func->_initial_line;
info.index = p_index;
Expand All @@ -3057,10 +3057,14 @@ GDScriptCompiler::FunctionLambdaInfo GDScriptCompiler::_get_function_replacement
info.default_arg_count = p_func->_default_arg_count;
info.sublambdas = _get_function_lambda_replacement_info(p_func, p_depth, p_parent_func);

GDScript::LambdaInfo *extra_info = main_script->lambda_info.getptr(p_func);
ERR_FAIL_NULL_V(info.script, info);
GDScript::LambdaInfo *extra_info = info.script->lambda_info.getptr(p_func);
if (extra_info != nullptr) {
info.capture_count = extra_info->capture_count;
info.use_self = extra_info->use_self;
} else {
info.capture_count = 0;
info.use_self = false;
}

return info;
Expand Down Expand Up @@ -3195,22 +3199,7 @@ Error GDScriptCompiler::compile(const GDScriptParser *p_parser, GDScript *p_scri

HashMap<GDScriptFunction *, GDScriptFunction *> func_ptr_replacements;
_get_function_ptr_replacements(func_ptr_replacements, old_lambda_info, &new_lambda_info);

{
MutexLock outer_lock(main_script->func_ptrs_to_update_mutex);
for (GDScript::UpdatableFuncPtr *updatable : main_script->func_ptrs_to_update) {
MutexLock inner_lock(updatable->mutex);
for (GDScriptFunction **func_ptr_ptr : updatable->ptrs) {
GDScriptFunction **replacement = func_ptr_replacements.getptr(*func_ptr_ptr);
if (replacement != nullptr) {
*func_ptr_ptr = *replacement;
} else {
// Probably a lambda from another reload, ignore.
*func_ptr_ptr = nullptr;
}
}
}
}
main_script->_recurse_replace_function_ptrs(func_ptr_replacements);

if (has_static_data && !root->annotated_static_unload) {
GDScriptCache::add_static_script(p_script);
Expand Down
20 changes: 10 additions & 10 deletions modules/gdscript/gdscript_compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@ class GDScriptCompiler {
GDScript *main_script = nullptr;

struct FunctionLambdaInfo {
GDScriptFunction *function;
GDScriptFunction *parent;
Ref<GDScript> script;
GDScriptFunction *function = nullptr;
GDScriptFunction *parent = nullptr;
GDScript *script = nullptr;
StringName name;
int line;
int index;
int depth;
int line = 0;
int index = 0;
int depth = 0;
//uint64_t code_hash;
//int code_size;
int capture_count;
int use_self;
int arg_count;
int default_arg_count;
int capture_count = 0;
bool use_self = false;
int arg_count = 0;
int default_arg_count = 0;
//Vector<GDScriptDataType> argument_types;
//GDScriptDataType return_type;
Vector<FunctionLambdaInfo> sublambdas;
Expand Down
32 changes: 6 additions & 26 deletions modules/gdscript/gdscript_lambda_callable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,20 +139,14 @@ void GDScriptLambdaCallable::call(const Variant **p_arguments, int p_argcount, V
}
}

GDScriptLambdaCallable::GDScriptLambdaCallable(Ref<GDScript> p_script, GDScriptFunction *p_function, const Vector<Variant> &p_captures) {
GDScriptLambdaCallable::GDScriptLambdaCallable(Ref<GDScript> p_script, GDScriptFunction *p_function, const Vector<Variant> &p_captures) :
function(p_function) {
ERR_FAIL_NULL(p_script.ptr());
ERR_FAIL_NULL(p_function);
script = p_script;
function = p_function;
captures = p_captures;

h = (uint32_t)hash_murmur3_one_64((uint64_t)this);

updatable_func_ptr_element = p_script->_add_func_ptr_to_update(&function);
}

GDScriptLambdaCallable::~GDScriptLambdaCallable() {
GDScript::_remove_func_ptr_to_update(updatable_func_ptr_element);
}

bool GDScriptLambdaSelfCallable::compare_equal(const CallableCustom *p_a, const CallableCustom *p_b) {
Expand Down Expand Up @@ -264,37 +258,23 @@ void GDScriptLambdaSelfCallable::call(const Variant **p_arguments, int p_argcoun
}
}

GDScriptLambdaSelfCallable::GDScriptLambdaSelfCallable(Ref<RefCounted> p_self, GDScriptFunction *p_function, const Vector<Variant> &p_captures) {
GDScriptLambdaSelfCallable::GDScriptLambdaSelfCallable(Ref<RefCounted> p_self, GDScriptFunction *p_function, const Vector<Variant> &p_captures) :
function(p_function) {
ERR_FAIL_NULL(p_self.ptr());
ERR_FAIL_NULL(p_function);
reference = p_self;
object = p_self.ptr();
function = p_function;
captures = p_captures;

h = (uint32_t)hash_murmur3_one_64((uint64_t)this);

GDScript *gds = p_function->get_script();
if (gds != nullptr) {
updatable_func_ptr_element = gds->_add_func_ptr_to_update(&function);
}
}

GDScriptLambdaSelfCallable::GDScriptLambdaSelfCallable(Object *p_self, GDScriptFunction *p_function, const Vector<Variant> &p_captures) {
GDScriptLambdaSelfCallable::GDScriptLambdaSelfCallable(Object *p_self, GDScriptFunction *p_function, const Vector<Variant> &p_captures) :
function(p_function) {
ERR_FAIL_NULL(p_self);
ERR_FAIL_NULL(p_function);
object = p_self;
function = p_function;
captures = p_captures;

h = (uint32_t)hash_murmur3_one_64((uint64_t)this);

GDScript *gds = p_function->get_script();
if (gds != nullptr) {
updatable_func_ptr_element = gds->_add_func_ptr_to_update(&function);
}
}

GDScriptLambdaSelfCallable::~GDScriptLambdaSelfCallable() {
GDScript::_remove_func_ptr_to_update(updatable_func_ptr_element);
}
Loading

0 comments on commit 1cb7d6f

Please sign in to comment.