Skip to content

Commit 0707d0d

Browse files
HolonProductionHolonProduction
committed
GDScript: Respect external refs when cleaning up cyclic refs
1 parent f0aeea2 commit 0707d0d

File tree

2 files changed

+257
-15
lines changed

2 files changed

+257
-15
lines changed

‎modules/gdscript/gdscript.cpp‎

Lines changed: 254 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030

3131
#include "gdscript.h"
3232

33+
#include "core/templates/local_vector.h"
34+
#include "core/templates/pair.h"
35+
#include "core/templates/rb_map.h"
36+
#include "core/templates/rb_set.h"
3337
#include "gdscript_analyzer.h"
3438
#include "gdscript_cache.h"
3539
#include "gdscript_compiler.h"
@@ -1385,6 +1389,28 @@ void GDScript::_collect_function_dependencies(GDScriptFunction *p_func, RBSet<GD
13851389
}
13861390
}
13871391

1392+
void GDScript::_collect_function_flat_dependencies(GDScriptFunction *p_func, RBMap<GDScript *, int> &p_dependencies, bool p_transitive) {
1393+
if (p_func == nullptr) {
1394+
return;
1395+
}
1396+
for (GDScriptFunction *lambda : p_func->lambdas) {
1397+
_collect_function_flat_dependencies(lambda, p_dependencies, p_transitive);
1398+
}
1399+
for (const Variant &V : p_func->constants) {
1400+
GDScript *scr = _get_gdscript_from_variant(V);
1401+
if (scr != nullptr) {
1402+
if (p_dependencies.has(scr)) {
1403+
p_dependencies[scr] += 1;
1404+
} else {
1405+
p_dependencies[scr] = 1;
1406+
if (p_transitive) {
1407+
scr->get_flat_dependencies(p_dependencies, true);
1408+
}
1409+
}
1410+
}
1411+
}
1412+
}
1413+
13881414
void GDScript::_collect_dependencies(RBSet<GDScript *> &p_dependencies, const GDScript *p_except) {
13891415
if (p_dependencies.has(this)) {
13901416
return;
@@ -1423,6 +1449,83 @@ void GDScript::_collect_dependencies(RBSet<GDScript *> &p_dependencies, const GD
14231449
}
14241450
}
14251451

1452+
void GDScript::get_flat_dependencies(RBMap<GDScript *, int> &r_res, bool transitive) {
1453+
//if (r_res.has(this)) {
1454+
// r_res[this] += 1;
1455+
// return;
1456+
//}
1457+
//r_res[this] = 1;
1458+
1459+
for (const KeyValue<StringName, GDScriptFunction *> &E : member_functions) {
1460+
_collect_function_flat_dependencies(E.value, r_res, transitive);
1461+
}
1462+
1463+
if (implicit_initializer) {
1464+
_collect_function_flat_dependencies(implicit_initializer, r_res, transitive);
1465+
}
1466+
1467+
if (implicit_ready) {
1468+
_collect_function_flat_dependencies(implicit_ready, r_res, transitive);
1469+
}
1470+
1471+
if (static_initializer) {
1472+
_collect_function_flat_dependencies(static_initializer, r_res, transitive);
1473+
}
1474+
1475+
for (KeyValue<StringName, MemberInfo> &E : member_indices) {
1476+
Ref<GDScript> ref = E.value.data_type.script_type_ref;
1477+
if (ref.is_valid()) {
1478+
if (r_res.has(*ref)) {
1479+
r_res[*ref] += 1;
1480+
} else {
1481+
r_res[*ref] = 1;
1482+
if (transitive) {
1483+
ref->get_flat_dependencies(r_res, true);
1484+
}
1485+
}
1486+
}
1487+
}
1488+
1489+
for (KeyValue<StringName, MemberInfo> &E : static_variables_indices) {
1490+
Ref<GDScript> ref = E.value.data_type.script_type_ref;
1491+
if (ref.is_valid()) {
1492+
if (r_res.has(*ref)) {
1493+
r_res[*ref] += 1;
1494+
} else {
1495+
r_res[*ref] = 1;
1496+
if (transitive) {
1497+
ref->get_flat_dependencies(r_res, true);
1498+
}
1499+
}
1500+
}
1501+
}
1502+
1503+
for (KeyValue<StringName, Ref<GDScript>> &E : subclasses) {
1504+
if (r_res.has(*E.value)) {
1505+
r_res[*E.value] += 1;
1506+
} else {
1507+
r_res[*E.value] = 1;
1508+
if (transitive) {
1509+
E.value->get_flat_dependencies(r_res, true);
1510+
}
1511+
}
1512+
}
1513+
1514+
for (const KeyValue<StringName, Variant> &E : constants) {
1515+
GDScript *scr = _get_gdscript_from_variant(E.value);
1516+
if (scr != nullptr) {
1517+
if (r_res.has(scr)) {
1518+
r_res[scr] += 1;
1519+
} else {
1520+
r_res[scr] = 1;
1521+
if (transitive) {
1522+
scr->get_flat_dependencies(r_res, true);
1523+
}
1524+
}
1525+
}
1526+
}
1527+
}
1528+
14261529
GDScript::GDScript() :
14271530
script_list(this) {
14281531
{
@@ -1546,6 +1649,53 @@ void GDScript::_recurse_replace_function_ptrs(const HashMap<GDScriptFunction *,
15461649
}
15471650
}
15481651

1652+
#define NOT_ON_STACK -2
1653+
#define ON_STACK -1
1654+
1655+
struct Data {
1656+
GDScript *script;
1657+
int index;
1658+
int lowlink;
1659+
int scc; // -2 not on stack, -1 on stack, >= 0 -> scc idx
1660+
};
1661+
1662+
void strongconnect(RBMap<GDScript *, int> &script_index, TightLocalVector<Data> &scripts, LocalVector<int> &stack, int &next_idx, int &next_scc_id, LocalVector<RBSet<GDScript *>> &sccs, int v) {
1663+
scripts[v].index = next_idx;
1664+
scripts[v].lowlink = next_idx;
1665+
scripts[v].scc = ON_STACK;
1666+
next_idx += 1;
1667+
stack.push_back(v);
1668+
1669+
RBMap<GDScript *, int> deps;
1670+
scripts[v].script->get_flat_dependencies(deps);
1671+
for (KeyValue<GDScript *, int> &dep : deps) {
1672+
if (dep.key == scripts[v].script) {
1673+
continue;
1674+
}
1675+
1676+
int w = script_index[dep.key];
1677+
if (scripts[w].index == -1) {
1678+
strongconnect(script_index, scripts, stack, next_idx, next_scc_id, sccs, w);
1679+
scripts[v].lowlink = MIN(scripts[v].lowlink, scripts[w].lowlink);
1680+
} else if (scripts[w].scc == ON_STACK) {
1681+
scripts[v].lowlink = MIN(scripts[v].lowlink, scripts[w].lowlink);
1682+
}
1683+
}
1684+
1685+
if (scripts[v].lowlink == scripts[v].index) {
1686+
RBSet<GDScript *> scc;
1687+
int head;
1688+
do {
1689+
head = stack[stack.size() - 1];
1690+
stack.resize(stack.size() - 1);
1691+
scc.insert(scripts[v].script);
1692+
scripts[v].scc = next_scc_id;
1693+
} while (head != v);
1694+
sccs.push_back(scc);
1695+
next_scc_id += 1;
1696+
}
1697+
}
1698+
15491699
void GDScript::clear(ClearData *p_clear_data) {
15501700
if (clearing) {
15511701
return;
@@ -1557,7 +1707,7 @@ void GDScript::clear(ClearData *p_clear_data) {
15571707
bool is_root = false;
15581708

15591709
// If `clear_data` is `nullptr`, it means that it's the root.
1560-
// The root is in charge to clear functions and scripts of itself and its dependencies
1710+
// The root is in charge to clear functions and scripts of itself.
15611711
if (clear_data == nullptr) {
15621712
clear_data = &data;
15631713
is_root = true;
@@ -1571,29 +1721,118 @@ void GDScript::clear(ClearData *p_clear_data) {
15711721
}
15721722

15731723
// If we're in the process of shutting things down then every single script will be cleared
1574-
// anyway, so we can safely skip this very costly operation.
1575-
if (!GDScriptLanguage::singleton->finishing) {
1576-
RBSet<GDScript *> must_clear_dependencies = get_must_clear_dependencies();
1577-
for (GDScript *E : must_clear_dependencies) {
1578-
clear_data->scripts.insert(E);
1579-
E->clear(clear_data);
1724+
// anyway, so we can safely skip this very costly operation. And if another root is clearing
1725+
// us up, it will take care of our deps as well.
1726+
if (is_root && !GDScriptLanguage::singleton->finishing) {
1727+
// Detect cyclic dependencies that are only referenced by this script.
1728+
// 1. The transitive deps of this script form a graph.
1729+
RBMap<GDScript *, int> nodes;
1730+
get_flat_dependencies(nodes, true);
1731+
if (!nodes.has(this)) {
1732+
nodes.insert(this, 0);
1733+
}
1734+
1735+
// This graph can contain cycles which keep themselves alive but are only needed for the script we are destructing right now.
1736+
// Instead of cycles we work with strongly connected components (SCC) of the graph, to account for overlapping cycles.
1737+
// 2. Identify SCCs using Tarjan's algorithm. This also topologically sorts the SCCs. (Refer to https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm for used terms.)
1738+
1739+
RBMap<GDScript *, int> script_index;
1740+
TightLocalVector<Data> scripts;
1741+
scripts.resize_uninitialized(nodes.size() + 1);
1742+
1743+
script_index[this] = 0;
1744+
scripts[0].script = this;
1745+
scripts[0].index = -1;
1746+
scripts[0].lowlink = -1;
1747+
scripts[0].scc = NOT_ON_STACK;
1748+
{
1749+
int i = 1;
1750+
for (KeyValue<GDScript *, int> &dep : nodes) {
1751+
script_index[dep.key] = i;
1752+
scripts[i].script = dep.key;
1753+
scripts[i].index = -1;
1754+
scripts[i].lowlink = -1;
1755+
scripts[i].scc = NOT_ON_STACK;
1756+
i++;
1757+
}
1758+
}
1759+
1760+
LocalVector<int> stack;
1761+
int index = 0;
1762+
int scc_idx = 0;
1763+
1764+
LocalVector<RBSet<GDScript *>> sccs;
1765+
1766+
// Since we derived the graph from transitive deps, we know it's connected. It's enough to start the DFS from this script and we will visit all nodes.
1767+
strongconnect(script_index, scripts, stack, index, scc_idx, sccs, 0);
1768+
1769+
// 3. For each sccs calculate: sum of refcounts - sum of internal refs - (sum of incoming refs from previous sccs which could be freed).
1770+
// If this count is > 0, there are references from outside of gdscript (e.g. user code) or other scripts that are not dependencies of this script.
1771+
1772+
TightLocalVector<int> counts;
1773+
counts.resize_initialized(scc_idx);
1774+
1775+
// Iterate backwards to start with the scc which has no incoming refs from other sccs and which contains this node. It is the root to unwinding this mess.
1776+
for (int i = sccs.size() - 1; i >= 0; i--) {
1777+
const RBSet<GDScript *> &scc = sccs[i];
1778+
1779+
RBMap<GDScript *, int> deps;
1780+
for (GDScript *scr : scc) {
1781+
counts[i] += scr->get_reference_count();
1782+
1783+
// Include script to also count refs to itself.
1784+
scr->get_flat_dependencies(deps);
1785+
}
1786+
1787+
for (KeyValue<GDScript *, int> &dep : deps) {
1788+
// The script is a transitive dep of this script so it was visited during scc detection. It's present in the index.
1789+
if (scripts[script_index[dep.key]].scc == i) {
1790+
// Script is part of this scc. Decrease refcount by internal refs.
1791+
counts[i] -= dep.value;
1792+
}
1793+
}
1794+
1795+
if (counts[i] > 0) {
1796+
// There are external refs. This scc can not be freed.
1797+
continue;
1798+
}
1799+
1800+
// Remove references from this scc.
1801+
for (KeyValue<GDScript *, int> &dep : deps) {
1802+
if (scripts[script_index[dep.key]].scc != i) {
1803+
counts[i] -= dep.value;
1804+
}
1805+
}
1806+
1807+
for (GDScript *scr : scc) {
1808+
clear_data->scripts.insert(scr);
1809+
scr->clear(clear_data);
1810+
}
15801811
}
15811812
}
15821813

1814+
//if (!GDScriptLanguage::singleton->finishing) {
1815+
// RBSet<GDScript *> must_clear_dependencies = get_must_clear_dependencies();
1816+
// for (GDScript *E : must_clear_dependencies) {
1817+
// clear_data->scripts.insert(E);
1818+
// E->clear(clear_data);
1819+
// }
1820+
//}
1821+
15831822
for (const KeyValue<StringName, GDScriptFunction *> &E : member_functions) {
15841823
clear_data->functions.insert(E.value);
15851824
}
15861825
member_functions.clear();
15871826

1588-
for (KeyValue<StringName, MemberInfo> &E : member_indices) {
1589-
clear_data->scripts.insert(E.value.data_type.script_type_ref);
1590-
E.value.data_type.script_type_ref = Ref<Script>();
1591-
}
1827+
//for (KeyValue<StringName, MemberInfo> &E : member_indices) {
1828+
// clear_data->scripts.insert(E.value.data_type.script_type_ref);
1829+
// E.value.data_type.script_type_ref = Ref<Script>();
1830+
//}
15921831

1593-
for (KeyValue<StringName, MemberInfo> &E : static_variables_indices) {
1594-
clear_data->scripts.insert(E.value.data_type.script_type_ref);
1595-
E.value.data_type.script_type_ref = Ref<Script>();
1596-
}
1832+
//for (KeyValue<StringName, MemberInfo> &E : static_variables_indices) {
1833+
// clear_data->scripts.insert(E.value.data_type.script_type_ref);
1834+
// E.value.data_type.script_type_ref = Ref<Script>();
1835+
//}
15971836
static_variables.clear();
15981837
static_variables_indices.clear();
15991838

‎modules/gdscript/gdscript.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#pragma once
3232

33+
#include "core/templates/rb_map.h"
3334
#include "gdscript_function.h"
3435

3536
#include "core/debugger/engine_debugger.h"
@@ -217,6 +218,7 @@ class GDScript : public Script {
217218

218219
GDScript *_get_gdscript_from_variant(const Variant &p_variant);
219220
void _collect_function_dependencies(GDScriptFunction *p_func, RBSet<GDScript *> &p_dependencies, const GDScript *p_except);
221+
void _collect_function_flat_dependencies(GDScriptFunction *p_func, RBMap<GDScript *, int> &p_dependencies, bool transitive);
220222
void _collect_dependencies(RBSet<GDScript *> &p_dependencies, const GDScript *p_except);
221223

222224
protected:
@@ -271,6 +273,7 @@ class GDScript : public Script {
271273
_FORCE_INLINE_ const GDScriptFunction *get_static_initializer() const { return static_initializer; }
272274

273275
RBSet<GDScript *> get_dependencies();
276+
void get_flat_dependencies(RBMap<GDScript *, int> &r_res, bool transitive = false);
274277
HashMap<GDScript *, RBSet<GDScript *>> get_all_dependencies();
275278
RBSet<GDScript *> get_must_clear_dependencies();
276279

0 commit comments

Comments
 (0)