Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Revert multiple GDScriptCache related PRs #95085

Closed
wants to merge 1 commit into from
Closed

GDScript: Revert multiple GDScriptCache related PRs #95085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 4 additions & 20 deletions modules/gdscript/gdscript.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,26 +767,10 @@ Error GDScript::reload(bool p_keep_state) {
if (source_path.is_empty()) {
source_path = get_path();
}
if (!source_path.is_empty()) {
if (GDScriptCache::get_cached_script(source_path).is_null()) {
MutexLock lock(GDScriptCache::singleton->mutex);
GDScriptCache::singleton->shallow_gdscript_cache[source_path] = Ref<GDScript>(this);
}
if (GDScriptCache::has_parser(source_path)) {
Error err = OK;
Ref<GDScriptParserRef> parser_ref = GDScriptCache::get_parser(source_path, GDScriptParserRef::EMPTY, err);
if (parser_ref.is_valid()) {
uint32_t source_hash;
if (!binary_tokens.is_empty()) {
source_hash = hash_djb2_buffer(binary_tokens.ptr(), binary_tokens.size());
} else {
source_hash = source.hash();
}
if (parser_ref->get_source_hash() != source_hash) {
GDScriptCache::remove_parser(source_path);
}
}
}
Ref<GDScript> cached_script = GDScriptCache::get_cached_script(source_path);
if (!source_path.is_empty() && cached_script.is_null()) {
MutexLock lock(GDScriptCache::singleton->mutex);
GDScriptCache::singleton->shallow_gdscript_cache[source_path] = Ref<GDScript>(this);
}
}

Expand Down
259 changes: 82 additions & 177 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 4 additions & 18 deletions modules/gdscript/gdscript_analyzer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,11 @@

class GDScriptAnalyzer {
GDScriptParser *parser = nullptr;

template <typename Fn>
class Finally {
Fn fn;

public:
Finally(Fn p_fn) :
fn(p_fn) {}
~Finally() {
fn();
}
};
HashMap<String, Ref<GDScriptParserRef>> depended_parsers;

const GDScriptParser::EnumNode *current_enum = nullptr;
GDScriptParser::LambdaNode *current_lambda = nullptr;
List<GDScriptParser::LambdaNode *> pending_body_resolution_lambdas;
HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>> external_class_parser_cache;
bool static_context = false;

// Tests for detecting invalid overloading of script members
Expand All @@ -65,7 +53,7 @@ class GDScriptAnalyzer {
Error check_native_member_name_conflict(const StringName &p_member_name, const GDScriptParser::Node *p_member_node, const StringName &p_native_type_string);
Error check_class_member_name_conflict(const GDScriptParser::ClassNode *p_class_node, const StringName &p_member_name, const GDScriptParser::Node *p_member_node);

void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list, GDScriptParser::Node *p_source);
void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list);

Error resolve_class_inheritance(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source = nullptr);
Error resolve_class_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive);
Expand Down Expand Up @@ -144,11 +132,8 @@ class GDScriptAnalyzer {
void mark_lambda_use_self();
void resolve_pending_lambda_bodies();
bool class_exists(const StringName &p_class) const;
Ref<GDScriptParserRef> get_parser_for(const String &p_path);
void reduce_identifier_from_base_set_class(GDScriptParser::IdentifierNode *p_identifier, GDScriptParser::DataType p_identifier_datatype);
Ref<GDScriptParserRef> ensure_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const String &p_context, const GDScriptParser::Node *p_source);
Ref<GDScriptParserRef> find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser);
Ref<GDScriptParserRef> find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser);
Ref<GDScript> get_depended_shallow_script(const String &p_path, Error &r_error);
#ifdef DEBUG_ENABLED
void is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope);
#endif
Expand All @@ -161,6 +146,7 @@ class GDScriptAnalyzer {
Error analyze();

Variant make_variable_default_value(GDScriptParser::VariableNode *p_variable);
const HashMap<String, Ref<GDScriptParserRef>> &get_depended_parsers();
static bool check_type_compatibility(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false, const GDScriptParser::Node *p_source_node = nullptr);

GDScriptAnalyzer(GDScriptParser *p_parser);
Expand Down
152 changes: 43 additions & 109 deletions modules/gdscript/gdscript_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,106 +38,96 @@
#include "core/io/file_access.h"
#include "core/templates/vector.h"

GDScriptParserRef::Status GDScriptParserRef::get_status() const {
return status;
}

String GDScriptParserRef::get_path() const {
return path;
bool GDScriptParserRef::is_valid() const {
return parser != nullptr;
}

uint32_t GDScriptParserRef::get_source_hash() const {
return source_hash;
GDScriptParserRef::Status GDScriptParserRef::get_status() const {
return status;
}

GDScriptParser *GDScriptParserRef::get_parser() {
if (parser == nullptr) {
parser = memnew(GDScriptParser);
}
GDScriptParser *GDScriptParserRef::get_parser() const {
return parser;
}

GDScriptAnalyzer *GDScriptParserRef::get_analyzer() {
if (analyzer == nullptr) {
analyzer = memnew(GDScriptAnalyzer(get_parser()));
analyzer = memnew(GDScriptAnalyzer(parser));
}
return analyzer;
}

Error GDScriptParserRef::raise_status(Status p_new_status) {
ERR_FAIL_COND_V(clearing, ERR_BUG);
ERR_FAIL_COND_V(parser == nullptr && status != EMPTY, ERR_BUG);
ERR_FAIL_NULL_V(parser, ERR_INVALID_DATA);

if (result != OK) {
return result;
}

while (result == OK && p_new_status > status) {
while (p_new_status > status) {
switch (status) {
case EMPTY: {
// Calling parse will clear the parser, which can destruct another GDScriptParserRef which can clear the last reference to the script with this path, calling remove_script, which clears this GDScriptParserRef.
// It's ok if its the first thing done here.
get_parser()->clear();
status = PARSED;
String remapped_path = ResourceLoader::path_remap(path);
if (remapped_path.get_extension().to_lower() == "gdc") {
Vector<uint8_t> tokens = GDScriptCache::get_binary_tokens(remapped_path);
source_hash = hash_djb2_buffer(tokens.ptr(), tokens.size());
result = get_parser()->parse_binary(tokens, path);
result = parser->parse_binary(GDScriptCache::get_binary_tokens(remapped_path), path);
} else {
String source = GDScriptCache::get_source_code(remapped_path);
source_hash = source.hash();
result = get_parser()->parse(source, path, false);
result = parser->parse(GDScriptCache::get_source_code(remapped_path), path, false);
}
} break;
case PARSED: {
status = INHERITANCE_SOLVED;
result = get_analyzer()->resolve_inheritance();
Error inheritance_result = get_analyzer()->resolve_inheritance();
if (result == OK) {
result = inheritance_result;
}
} break;
case INHERITANCE_SOLVED: {
status = INTERFACE_SOLVED;
result = get_analyzer()->resolve_interface();
Error interface_result = get_analyzer()->resolve_interface();
if (result == OK) {
result = interface_result;
}
} break;
case INTERFACE_SOLVED: {
status = FULLY_SOLVED;
result = get_analyzer()->resolve_body();
Error body_result = get_analyzer()->resolve_body();
if (result == OK) {
result = body_result;
}
} break;
case FULLY_SOLVED: {
return result;
}
}
if (result != OK) {
return result;
}
}

return result;
}

void GDScriptParserRef::clear() {
if (clearing) {
if (cleared) {
return;
}
clearing = true;

GDScriptParser *lparser = parser;
GDScriptAnalyzer *lanalyzer = analyzer;

parser = nullptr;
analyzer = nullptr;
status = EMPTY;
result = OK;
source_hash = 0;
cleared = true;

clearing = false;

if (lanalyzer != nullptr) {
memdelete(lanalyzer);
if (parser != nullptr) {
memdelete(parser);
}

if (lparser != nullptr) {
memdelete(lparser);
if (analyzer != nullptr) {
memdelete(analyzer);
}
}

GDScriptParserRef::~GDScriptParserRef() {
clear();
if (!abandoned) {
GDScriptCache::remove_parser(path);
}

MutexLock lock(GDScriptCache::singleton->mutex);
GDScriptCache::singleton->parser_map.erase(path);
}

GDScriptCache *GDScriptCache::singleton = nullptr;
Expand All @@ -158,11 +148,6 @@ void GDScriptCache::move_script(const String &p_from, const String &p_to) {
}
singleton->parser_map.erase(p_from);

if (singleton->parser_inverse_dependencies.has(p_from) && !p_from.is_empty()) {
singleton->parser_inverse_dependencies[p_to] = singleton->parser_inverse_dependencies[p_from];
}
singleton->parser_inverse_dependencies.erase(p_from);

if (singleton->shallow_gdscript_cache.has(p_from) && !p_from.is_empty()) {
singleton->shallow_gdscript_cache[p_to] = singleton->shallow_gdscript_cache[p_from];
}
Expand All @@ -185,23 +170,11 @@ void GDScriptCache::remove_script(const String &p_path) {
return;
}

if (HashMap<String, Vector<ObjectID>>::Iterator E = singleton->abandoned_parser_map.find(p_path)) {
for (ObjectID parser_ref_id : E->value) {
Ref<GDScriptParserRef> parser_ref{ ObjectDB::get_instance(parser_ref_id) };
if (parser_ref.is_valid()) {
parser_ref->clear();
}
}
}

singleton->abandoned_parser_map.erase(p_path);

if (singleton->parser_map.has(p_path)) {
singleton->parser_map[p_path]->clear();
singleton->parser_map.erase(p_path);
}

remove_parser(p_path);

singleton->dependencies.erase(p_path);
singleton->shallow_gdscript_cache.erase(p_path);
singleton->full_gdscript_cache.erase(p_path);
Expand All @@ -212,7 +185,6 @@ Ref<GDScriptParserRef> GDScriptCache::get_parser(const String &p_path, GDScriptP
Ref<GDScriptParserRef> ref;
if (!p_owner.is_empty()) {
singleton->dependencies[p_owner].insert(p_path);
singleton->parser_inverse_dependencies[p_path].insert(p_owner);
}
if (singleton->parser_map.has(p_path)) {
ref = Ref<GDScriptParserRef>(singleton->parser_map[p_path]);
Expand All @@ -226,7 +198,9 @@ Ref<GDScriptParserRef> GDScriptCache::get_parser(const String &p_path, GDScriptP
r_error = ERR_FILE_NOT_FOUND;
return ref;
}
GDScriptParser *parser = memnew(GDScriptParser);
ref.instantiate();
ref->parser = parser;
ref->path = p_path;
singleton->parser_map[p_path] = ref.ptr();
}
Expand All @@ -235,31 +209,6 @@ Ref<GDScriptParserRef> GDScriptCache::get_parser(const String &p_path, GDScriptP
return ref;
}

bool GDScriptCache::has_parser(const String &p_path) {
MutexLock lock(singleton->mutex);
return singleton->parser_map.has(p_path);
}

void GDScriptCache::remove_parser(const String &p_path) {
MutexLock lock(singleton->mutex);

if (singleton->parser_map.has(p_path)) {
GDScriptParserRef *parser_ref = singleton->parser_map[p_path];
parser_ref->abandoned = true;
singleton->abandoned_parser_map[p_path].push_back(parser_ref->get_instance_id());
}

// Can't clear the parser because some other parser might be currently using it in the chain of calls.
singleton->parser_map.erase(p_path);

// Have to copy while iterating, because parser_inverse_dependencies is modified.
HashSet<String> ideps = singleton->parser_inverse_dependencies[p_path];
singleton->parser_inverse_dependencies.erase(p_path);
for (String idep_path : ideps) {
remove_parser(idep_path);
}
}

String GDScriptCache::get_source_code(const String &p_path) {
Vector<uint8_t> source_file;
Error err;
Expand Down Expand Up @@ -450,33 +399,18 @@ void GDScriptCache::clear() {
}
singleton->cleared = true;

singleton->parser_inverse_dependencies.clear();

for (const KeyValue<String, Vector<ObjectID>> &KV : singleton->abandoned_parser_map) {
for (ObjectID parser_ref_id : KV.value) {
Ref<GDScriptParserRef> parser_ref{ ObjectDB::get_instance(parser_ref_id) };
if (parser_ref.is_valid()) {
parser_ref->clear();
}
}
}

singleton->abandoned_parser_map.clear();

RBSet<Ref<GDScriptParserRef>> parser_map_refs;
for (KeyValue<String, GDScriptParserRef *> &E : singleton->parser_map) {
parser_map_refs.insert(E.value);
}

singleton->parser_map.clear();

for (Ref<GDScriptParserRef> &E : parser_map_refs) {
if (E.is_valid()) {
if (E.is_valid())
E->clear();
}
}

parser_map_refs.clear();
singleton->parser_map.clear();
singleton->shallow_gdscript_cache.clear();
singleton->full_gdscript_cache.clear();
}
Expand Down
14 changes: 3 additions & 11 deletions modules/gdscript/gdscript_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,14 @@ class GDScriptParserRef : public RefCounted {
Status status = EMPTY;
Error result = OK;
String path;
uint32_t source_hash = 0;
bool clearing = false;
bool abandoned = false;
bool cleared = false;

friend class GDScriptCache;
friend class GDScript;

public:
bool is_valid() const;
Status get_status() const;
String get_path() const;
uint32_t get_source_hash() const;
GDScriptParser *get_parser();
GDScriptParser *get_parser() const;
GDScriptAnalyzer *get_analyzer();
Error raise_status(Status p_new_status);
void clear();
Expand All @@ -80,12 +76,10 @@ class GDScriptParserRef : public RefCounted {
class GDScriptCache {
// String key is full path.
HashMap<String, GDScriptParserRef *> parser_map;
HashMap<String, Vector<ObjectID>> abandoned_parser_map;
HashMap<String, Ref<GDScript>> shallow_gdscript_cache;
HashMap<String, Ref<GDScript>> full_gdscript_cache;
HashMap<String, Ref<GDScript>> static_gdscript_cache;
HashMap<String, HashSet<String>> dependencies;
HashMap<String, HashSet<String>> parser_inverse_dependencies;

friend class GDScript;
friend class GDScriptParserRef;
Expand All @@ -101,8 +95,6 @@ class GDScriptCache {
static void move_script(const String &p_from, const String &p_to);
static void remove_script(const String &p_path);
static Ref<GDScriptParserRef> get_parser(const String &p_path, GDScriptParserRef::Status status, Error &r_error, const String &p_owner = String());
static bool has_parser(const String &p_path);
static void remove_parser(const String &p_path);
static String get_source_code(const String &p_path);
static Vector<uint8_t> get_binary_tokens(const String &p_path);
static Ref<GDScript> get_shallow_script(const String &p_path, Error &r_error, const String &p_owner = String());
Expand Down
Loading
Loading