diff --git a/core/io/config_file.cpp b/core/io/config_file.cpp index 273c165b1076..a50d438181bd 100644 --- a/core/io/config_file.cpp +++ b/core/io/config_file.cpp @@ -243,7 +243,7 @@ Error ConfigFile::load_encrypted_pass(const String &p_path, const String &p_pass Error ConfigFile::_internal_load(const String &p_path, FileAccess *f) { VariantParser::StreamFile stream; - stream.f = f; + stream.set_file(f, VariantParser::Stream::READAHEAD_ENABLED); Error err = _parse(p_path, &stream); diff --git a/core/io/resource_importer.cpp b/core/io/resource_importer.cpp index 9c67f6fc9336..96cab2c0e2b2 100644 --- a/core/io/resource_importer.cpp +++ b/core/io/resource_importer.cpp @@ -39,19 +39,15 @@ bool ResourceFormatImporter::SortImporterByName::operator()(const Ref *r_paths) { - Error err; - FileAccess *f = FileAccess::open(p_path + ".import", FileAccess::READ, &err); - - if (!f) { + VariantParser::StreamFile stream; + Error err = stream.open_file(p_path + ".import"); + if (err != OK) { return; } - VariantParser::StreamFile stream; - stream.f = f; - String assign; Variant value; VariantParser::Tag next_tag; @@ -266,11 +254,9 @@ void ResourceFormatImporter::get_internal_resource_path_list(const String &p_pat err = VariantParser::parse_tag_assign_eof(&stream, lines, error_text, next_tag, assign, value, nullptr, true); if (err == ERR_FILE_EOF) { - memdelete(f); return; } else if (err != OK) { ERR_PRINT("ResourceFormatImporter::get_internal_resource_path_list - " + p_path + ".import:" + itos(lines) + " error: " + error_text); - memdelete(f); return; } @@ -284,7 +270,6 @@ void ResourceFormatImporter::get_internal_resource_path_list(const String &p_pat break; } } - memdelete(f); } String ResourceFormatImporter::get_import_group_file(const String &p_path) const { diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp index 068482e66cee..c022169b73d0 100644 --- a/core/io/resource_loader.cpp +++ b/core/io/resource_loader.cpp @@ -768,13 +768,9 @@ String ResourceLoader::_path_remap(const String &p_path, bool *r_translation_rem new_path = path_remaps[new_path]; } else { // Try file remap. - Error err; - FileAccess *f = FileAccess::open(new_path + ".remap", FileAccess::READ, &err); - - if (f) { - VariantParser::StreamFile stream; - stream.f = f; - + VariantParser::StreamFile stream; + Error err = stream.open_file(new_path + ".remap"); + if (err == OK) { String assign; Variant value; VariantParser::Tag next_tag; @@ -801,8 +797,6 @@ String ResourceLoader::_path_remap(const String &p_path, bool *r_translation_rem break; } } - - memdelete(f); } } diff --git a/core/project_settings.cpp b/core/project_settings.cpp index d21622e14d8d..947bc779627f 100644 --- a/core/project_settings.cpp +++ b/core/project_settings.cpp @@ -581,18 +581,14 @@ Error ProjectSettings::_load_settings_binary(const String &p_path) { } Error ProjectSettings::_load_settings_text(const String &p_path) { - Error err; - FileAccess *f = FileAccess::open(p_path, FileAccess::READ, &err); - - if (!f) { + VariantParser::StreamFile stream; + Error err = stream.open_file(p_path); + if (err != OK) { // FIXME: Above 'err' error code is ERR_FILE_CANT_OPEN if the file is missing // This needs to be streamlined if we want decent error reporting return ERR_FILE_NOT_FOUND; } - VariantParser::StreamFile stream; - stream.f = f; - String assign; Variant value; VariantParser::Tag next_tag; @@ -609,7 +605,6 @@ Error ProjectSettings::_load_settings_text(const String &p_path) { err = VariantParser::parse_tag_assign_eof(&stream, lines, error_text, next_tag, assign, value, nullptr, true); if (err == ERR_FILE_EOF) { - memdelete(f); // If we're loading a project.godot from source code, we can operate some // ProjectSettings conversions if need be. _convert_to_last_version(config_version); @@ -617,7 +612,6 @@ Error ProjectSettings::_load_settings_text(const String &p_path) { return OK; } else if (err != OK) { ERR_PRINT("Error parsing " + p_path + " at line " + itos(lines) + ": " + error_text + " File might be corrupted."); - memdelete(f); return err; } @@ -625,7 +619,6 @@ Error ProjectSettings::_load_settings_text(const String &p_path) { if (section == String() && assign == "config_version") { config_version = value; if (config_version > CONFIG_VERSION) { - memdelete(f); ERR_FAIL_V_MSG(ERR_FILE_CANT_OPEN, vformat("Can't open project at '%s', its `config_version` (%d) is from a more recent and incompatible version of the engine. Expected config version: %d.", p_path, config_version, CONFIG_VERSION)); } } else { diff --git a/core/variant_parser.cpp b/core/variant_parser.cpp index e620c883ced4..93ccef37f4dd 100644 --- a/core/variant_parser.cpp +++ b/core/variant_parser.cpp @@ -35,6 +35,7 @@ #include "core/os/keyboard.h" #include "core/string_buffer.h" +// Keep this function non-virtual, and as optimized as possible for the common case. CharType VariantParser::Stream::get_char() { // is within buffer? if (readahead_pointer < readahead_filled) { @@ -42,31 +43,148 @@ CharType VariantParser::Stream::get_char() { } // attempt to readahead - readahead_filled = _read_buffer(readahead_buffer, readahead_enabled ? READAHEAD_SIZE : 1); + readahead_filled = _read_buffer(readahead_buffer, readahead_size); if (readahead_filled) { readahead_pointer = 0; } else { // EOF readahead_pointer = 1; - eof = true; + _eof = true; return 0; } return get_char(); } bool VariantParser::Stream::is_eof() const { - if (readahead_enabled) { - return eof; + return _eof; +} + +uint32_t VariantParser::Stream::get_readahead_offset() const { + DEV_ASSERT(!_eof || ((readahead_filled - readahead_pointer) == 0)); + // Uncomment the next line if the assert is being hit. + // if (_eof) {return 0;} + return readahead_filled - readahead_pointer; +} + +void VariantParser::Stream::invalidate_readahead(bool p_eof) { + readahead_filled = 0; + readahead_pointer = p_eof ? 1 : 0; + _eof = p_eof; +} + +Error VariantParser::StreamFile::open_file(const String &p_path) { + Error err; + FileAccess *f = FileAccess::open(p_path, FileAccess::READ, &err); + + if (!f) { + // double check we are not sending an OK in case of failure. + if (err == OK) { + err = FAILED; + } + return err; + } + + if (err == OK) { + // Always use readahead for owned files. + // This is always safe, because the file pointer etc cannot be changed + // externally, and the chief reason for having owned files. + set_file(f, READAHEAD_ENABLED); + _is_owned_file = true; + } + + return err; +} + +Error VariantParser::StreamFile::close_file() { + if (_is_owned_file) { + if (_file) { + memdelete(_file); + _file = nullptr; + _is_owned_file = false; + invalidate_readahead(); + return OK; + } + // This should probably never occur, but just in case... + _is_owned_file = false; + } + + return FAILED; +} + +VariantParser::StreamFile::~StreamFile() { + // free any owned file + close_file(); +} + +void VariantParser::StreamFile::set_file(FileAccess *p_file, Readahead p_readahead) { + // free any existing owned file + close_file(); + + bool use_readahead = p_readahead == READAHEAD_ENABLED; + + // noop? + if ((p_file == _file) && (use_readahead == readahead_enabled())) { + return; + } + + _file = p_file; + + // set_file defaults to a non-owned file, + // if we are setting an owned file we must set this bool + // immediately after set_file(). + _is_owned_file = false; + + readahead_size = use_readahead ? READAHEAD_SIZE : 1; + invalidate_readahead(); + + // Files should be opened BEFORE calling set_file. + ERR_FAIL_COND(_file && !_file->is_open()); +} + +void VariantParser::StreamFile::invalidate_readahead() { + bool eof = true; + if (_file) { + bool open = _file->is_open(); + eof = open ? _file->eof_reached() : false; + + VariantParser::Stream::invalidate_readahead(eof); +#ifdef DEV_ENABLED + _readahead_start_source_pos = open ? _file->get_position() : 0; + _readahead_end_source_pos = _readahead_start_source_pos; +#endif + } else { + VariantParser::Stream::invalidate_readahead(true); +#ifdef DEV_ENABLED + _readahead_start_source_pos = 0; + _readahead_end_source_pos = 0; +#endif } - return _is_eof(); } uint32_t VariantParser::StreamFile::_read_buffer(CharType *p_buffer, uint32_t p_num_chars) { // The buffer is assumed to include at least one character (for null terminator) - ERR_FAIL_COND_V(!p_num_chars, 0); + DEV_ASSERT(p_num_chars); + ERR_FAIL_NULL_V_MSG(_file, 0, "Attempting to read from NULL file."); + +#ifdef DEV_ENABLED + if (readahead_enabled()) { + _readahead_start_source_pos = _file->get_position(); + if (_readahead_start_source_pos != _readahead_end_source_pos) { + ERR_PRINT_ONCE("StreamFile out of sync. HIGH PRIORITY bug, please report on github."); + return 0; + } + } +#endif uint8_t *temp = (uint8_t *)alloca(p_num_chars); - uint64_t num_read = f->get_buffer(temp, p_num_chars); + uint64_t num_read = _file->get_buffer(temp, p_num_chars); + +#ifdef DEV_ENABLED + if (readahead_enabled()) { + _readahead_end_source_pos = _file->get_position(); + } +#endif + ERR_FAIL_COND_V(num_read == UINT64_MAX, 0); // translate to wchar @@ -83,19 +201,32 @@ bool VariantParser::StreamFile::is_utf8() const { } bool VariantParser::StreamFile::_is_eof() const { - return f->eof_reached(); + ERR_FAIL_NULL_V(_file, true); + return _file->eof_reached(); +} + +uint64_t VariantParser::StreamFile::get_position() const { + ERR_FAIL_NULL_V(_file, -1); + + // Note this assumes that get_position() returns get_length() when EOF + // is reached. Watch for bugs here. + return _file->get_position() - get_readahead_offset(); +} + +uint64_t VariantParser::StreamString::get_position() const { + return _pos - get_readahead_offset(); } uint32_t VariantParser::StreamString::_read_buffer(CharType *p_buffer, uint32_t p_num_chars) { // The buffer is assumed to include at least one character (for null terminator) - ERR_FAIL_COND_V(!p_num_chars, 0); + DEV_ASSERT(p_num_chars); - int available = MAX(s.length() - pos, 0); + int available = MAX(s.length() - _pos, 0); if (available >= (int)p_num_chars) { const CharType *src = s.ptr(); - src += pos; + src += _pos; memcpy(p_buffer, src, p_num_chars * sizeof(CharType)); - pos += p_num_chars; + _pos += p_num_chars; return p_num_chars; } @@ -103,9 +234,9 @@ uint32_t VariantParser::StreamString::_read_buffer(CharType *p_buffer, uint32_t // going to reach EOF if (available) { const CharType *src = s.ptr(); - src += pos; + src += _pos; memcpy(p_buffer, src, available * sizeof(CharType)); - pos += available; + _pos += available; } // add a zero @@ -119,7 +250,7 @@ bool VariantParser::StreamString::is_utf8() const { } bool VariantParser::StreamString::_is_eof() const { - return pos > s.length(); + return _pos > s.length(); } ///////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/core/variant_parser.h b/core/variant_parser.h index 124287ed8a41..8d6071c06878 100644 --- a/core/variant_parser.h +++ b/core/variant_parser.h @@ -31,6 +31,7 @@ #ifndef VARIANT_PARSER_H #define VARIANT_PARSER_H +#include "core/error_list.h" #include "core/os/file_access.h" #include "core/resource.h" #include "core/variant.h" @@ -38,23 +39,45 @@ class VariantParser { public: struct Stream { - private: - enum { READAHEAD_SIZE = 2048 }; - CharType readahead_buffer[READAHEAD_SIZE]; + protected: + // The readahead buffer is set by derived classes, + // and can be a single character to effectively turn off readahead. + CharType *readahead_buffer = nullptr; + uint32_t readahead_size = 0; + + // The eof is NOT necessarily the source (e.g. file) eof, + // because the readahead will reach source eof BEFORE + // the stream catches up. + bool _eof = false; + + // Number of characters we have read through already + // in the buffer (points to the next one to read). uint32_t readahead_pointer = 0; + + // Characters filled in the buffer uint32_t readahead_filled = 0; - bool eof = false; - protected: - bool readahead_enabled = true; virtual uint32_t _read_buffer(CharType *p_buffer, uint32_t p_num_chars) = 0; virtual bool _is_eof() const = 0; + void invalidate_readahead(bool p_eof); + bool readahead_enabled() const { return readahead_size != 1; } + + // The offset between the current read position (usually behind) and the current + // file position (usually ahead), due to the readahead cache. + uint32_t get_readahead_offset() const; + public: + enum Readahead { + READAHEAD_DISABLED, + READAHEAD_ENABLED, + }; + CharType saved; CharType get_char(); virtual bool is_utf8() const = 0; + virtual uint64_t get_position() const = 0; bool is_eof() const; Stream() : @@ -63,35 +86,86 @@ class VariantParser { }; struct StreamFile : public Stream { + private: + enum { READAHEAD_SIZE = 2048 }; + +#ifdef DEV_ENABLED + // These are used exclusively to check for out of sync errors. + uint64_t _readahead_start_source_pos = 0; + uint64_t _readahead_end_source_pos = 0; +#endif + FileAccess *_file = nullptr; + + // Owned files are opened using open_file() rather than set_file(). + // They cannot be modified from outside StreamFile, + // so are safe for readahead. + // They will also be automatically closed when StreamFile + // is destructed, or can optionally be closed using close_file(). + bool _is_owned_file = false; + + // Put buffer last in struct to keep the rest in cache + CharType _buffer[READAHEAD_SIZE]; + + void invalidate_readahead(); + protected: virtual uint32_t _read_buffer(CharType *p_buffer, uint32_t p_num_chars); virtual bool _is_eof() const; public: - FileAccess *f; + // NOTE: + // Whenever possible, prefer to use the open_file() / close_file() functions + // for the StreamFile to have internal ownership of the FileAccess. + + // If you intend to manipulate the file / file position from outside StreamFile, + // make sure to either set readahead to false (to avoid syncing errors), + // otherwise the readahead will GET OUT OF SYNC, and corrupt files may ensue. + + // Readahead is approx twice as fast, so should be used when possible. + void set_file(FileAccess *p_file, Readahead p_readahead = READAHEAD_DISABLED); + + // Rather than have external ownership, it is safer to allow StreamFile + // internal ownership of the file, to prevent external changes to the file + // (via seeking / closing etc) which would make the readahead lose sync. + Error open_file(const String &p_path); + Error close_file(); virtual bool is_utf8() const; - StreamFile(bool p_readahead_enabled = true) { - f = nullptr; - readahead_enabled = p_readahead_enabled; + virtual uint64_t get_position() const; + + StreamFile() { + readahead_buffer = _buffer; + readahead_size = READAHEAD_SIZE; } + virtual ~StreamFile(); }; struct StreamString : public Stream { private: - int pos; + // Keep the buffer as compact as possible for String, + // as it will have little effect, but to prevent the need for a virtual get_char() function. + enum { READAHEAD_SIZE = 8 }; + int _pos; + + public: + String s; + + private: + // Put buffer last in struct to keep the rest in cache + CharType _buffer[READAHEAD_SIZE]; protected: virtual uint32_t _read_buffer(CharType *p_buffer, uint32_t p_num_chars); virtual bool _is_eof() const; public: - String s; - virtual bool is_utf8() const; - StreamString(bool p_readahead_enabled = true) { - pos = 0; - readahead_enabled = p_readahead_enabled; + virtual uint64_t get_position() const; + + StreamString() { + readahead_buffer = _buffer; + readahead_size = READAHEAD_SIZE; + _pos = 0; } }; diff --git a/editor/editor_file_system.cpp b/editor/editor_file_system.cpp index b3fbb973ca7b..6e6699ebf177 100644 --- a/editor/editor_file_system.cpp +++ b/editor/editor_file_system.cpp @@ -342,16 +342,13 @@ bool EditorFileSystem::_test_for_reimport(const String &p_path, bool p_only_impo return true; } - Error err; - FileAccess *f = FileAccess::open(p_path + ".import", FileAccess::READ, &err); - - if (!f) { //no import file, do reimport + VariantParser::StreamFile stream; + Error err = stream.open_file(p_path + ".import"); + if (err != OK) { + // no import file, do reimport return true; } - VariantParser::StreamFile stream; - stream.f = f; - String assign; Variant value; VariantParser::Tag next_tag; @@ -377,7 +374,6 @@ bool EditorFileSystem::_test_for_reimport(const String &p_path, bool p_only_impo break; } else if (err != OK) { ERR_PRINT("ResourceFormatImporter::load - '" + p_path + ".import:" + itos(lines) + "' error '" + error_text + "'."); - memdelete(f); return false; //parse error, try reimport manually (Avoid reimport loop on broken file) } @@ -404,7 +400,7 @@ bool EditorFileSystem::_test_for_reimport(const String &p_path, bool p_only_impo } } - memdelete(f); + stream.close_file(); if (importer_name == "keep") { return false; //keep mode, do not reimport @@ -412,14 +408,13 @@ bool EditorFileSystem::_test_for_reimport(const String &p_path, bool p_only_impo // Read the md5's from a separate file (so the import parameters aren't dependent on the file version String base_path = ResourceFormatImporter::get_singleton()->get_import_base_path(p_path); - FileAccess *md5s = FileAccess::open(base_path + ".md5", FileAccess::READ, &err); - if (!md5s) { // No md5's stored for this resource + VariantParser::StreamFile md5_stream; + err = md5_stream.open_file(base_path + ".md5"); + if (err != OK) { + // No md5's stored for this resource return true; } - VariantParser::StreamFile md5_stream; - md5_stream.f = md5s; - while (true) { assign = Variant(); next_tag.fields.clear(); @@ -431,7 +426,6 @@ bool EditorFileSystem::_test_for_reimport(const String &p_path, bool p_only_impo break; } else if (err != OK) { ERR_PRINT("ResourceFormatImporter::load - '" + p_path + ".import.md5:" + itos(lines) + "' error '" + error_text + "'."); - memdelete(md5s); return false; // parse error } if (assign != String()) { @@ -444,7 +438,7 @@ bool EditorFileSystem::_test_for_reimport(const String &p_path, bool p_only_impo } } } - memdelete(md5s); + md5_stream.close_file(); //imported files are gone, reimport for (List::Element *E = to_check.front(); E; E = E->next()) { diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index 0f4f8cc890ef..2095f7ca96d2 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -3737,7 +3737,20 @@ void EditorNode::open_request(const String &p_path) { } } +#ifdef TOOLS_ENABLED +#if 0 +#define GODOT_TIME_SCENE_LOADING +#endif +#endif + +#ifdef GODOT_TIME_SCENE_LOADING + uint32_t before = OS::get_singleton()->get_ticks_msec(); + load_scene(p_path); + uint32_t after = OS::get_singleton()->get_ticks_msec(); + print_verbose("EditorNode::load_scene() took " + itos(after - before) + " ms."); +#else load_scene(p_path); // as it will be opened in separate tab +#endif } void EditorNode::request_instance_scene(const String &p_path) { diff --git a/scene/resources/resource_format_text.cpp b/scene/resources/resource_format_text.cpp index a5c8c0e5a48b..755ca91b6a0f 100644 --- a/scene/resources/resource_format_text.cpp +++ b/scene/resources/resource_format_text.cpp @@ -629,8 +629,7 @@ void ResourceInteractiveLoaderText::set_translation_remapped(bool p_remapped) { translation_remapped = p_remapped; } -ResourceInteractiveLoaderText::ResourceInteractiveLoaderText() : - stream(false) { +ResourceInteractiveLoaderText::ResourceInteractiveLoaderText() { translation_remapped = false; } @@ -684,7 +683,7 @@ void ResourceInteractiveLoaderText::get_dependencies(FileAccess *p_f, List &p_map) { - open(p_f, true); + open(p_f, true, false); ERR_FAIL_COND_V(error != OK, error); ignore_resource_parsing = true; //FileAccess @@ -786,13 +785,13 @@ Error ResourceInteractiveLoaderText::rename_dependencies(FileAccess *p_f, const return OK; } -void ResourceInteractiveLoaderText::open(FileAccess *p_f, bool p_skip_first_tag) { +void ResourceInteractiveLoaderText::open(FileAccess *p_f, bool p_skip_first_tag, bool p_readahead) { error = OK; lines = 1; f = p_f; - stream.f = f; + stream.set_file(f, p_readahead ? VariantParser::Stream::READAHEAD_ENABLED : VariantParser::Stream::READAHEAD_DISABLED); is_scene = false; ignore_resource_parsing = false; resource_current = 0; @@ -1133,7 +1132,7 @@ String ResourceInteractiveLoaderText::recognize(FileAccess *p_f) { lines = 1; f = p_f; - stream.f = f; + stream.set_file(f, VariantParser::Stream::READAHEAD_ENABLED); ignore_resource_parsing = true; diff --git a/scene/resources/resource_format_text.h b/scene/resources/resource_format_text.h index 13d7b2e3e493..6311e63a9f18 100644 --- a/scene/resources/resource_format_text.h +++ b/scene/resources/resource_format_text.h @@ -116,7 +116,7 @@ class ResourceInteractiveLoaderText : public ResourceInteractiveLoader { virtual int get_stage_count() const; virtual void set_translation_remapped(bool p_remapped); - void open(FileAccess *p_f, bool p_skip_first_tag = false); + void open(FileAccess *p_f, bool p_skip_first_tag = false, bool p_readahead = true); String recognize(FileAccess *p_f); void get_dependencies(FileAccess *p_f, List *p_dependencies, bool p_add_types); Error rename_dependencies(FileAccess *p_f, const String &p_path, const Map &p_map);