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

Polish & fix editor help cache generation #84354

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Polish & fix editor help cache generation #84354

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
17 changes: 16 additions & 1 deletion core/object/class_db.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,24 @@ void ClassDB::get_class_list(List<StringName> *p_classes) {
p_classes->push_back(E.key);
}

p_classes->sort();
p_classes->sort_custom<StringName::AlphCompare>();
}

#ifdef TOOLS_ENABLED
void ClassDB::get_extensions_class_list(List<StringName> *p_classes) {
OBJTYPE_RLOCK;

for (const KeyValue<StringName, ClassInfo> &E : classes) {
if (E.value.api != API_EXTENSION && E.value.api != API_EDITOR_EXTENSION) {
continue;
}
p_classes->push_back(E.key);
}

p_classes->sort_custom<StringName::AlphCompare>();
}
#endif

void ClassDB::get_inheriters_from_class(const StringName &p_class, List<StringName> *p_classes) {
OBJTYPE_RLOCK;

Expand Down
3 changes: 3 additions & 0 deletions core/object/class_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@ class ClassDB {
}

static void get_class_list(List<StringName> *p_classes);
#ifdef TOOLS_ENABLED
static void get_extensions_class_list(List<StringName> *p_classes);
#endif
static void get_inheriters_from_class(const StringName &p_class, List<StringName> *p_classes);
static void get_direct_inheriters_from_class(const StringName &p_class, List<StringName> *p_classes);
static StringName get_parent_class_nocheck(const StringName &p_class);
Expand Down
28 changes: 17 additions & 11 deletions editor/doc_tools.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,20 +355,27 @@ static Variant get_documentation_default_value(const StringName &p_class_name, c
return default_value;
}

void DocTools::generate(bool p_basic_types) {
void DocTools::generate(BitField<GenerateFlags> p_flags) {
// This may involve instantiating classes that are only usable from the main thread
// (which is in fact the case of the core API).
ERR_FAIL_COND(!Thread::is_main_thread());

// Add ClassDB-exposed classes.
{
List<StringName> classes;
ClassDB::get_class_list(&classes);
classes.sort_custom<StringName::AlphCompare>();
// Move ProjectSettings, so that other classes can register properties there.
classes.move_to_back(classes.find("ProjectSettings"));
if (p_flags.has_flag(GENERATE_FLAG_EXTENSION_CLASSES_ONLY)) {
ClassDB::get_extensions_class_list(&classes);
} else {
ClassDB::get_class_list(&classes);
// Move ProjectSettings, so that other classes can register properties there.
classes.move_to_back(classes.find("ProjectSettings"));
}

bool skip_setter_getter_methods = true;

// Populate documentation data for each exposed class.
while (classes.size()) {
String name = classes.front()->get();
const String &name = classes.front()->get();
if (!ClassDB::is_class_exposed(name)) {
print_verbose(vformat("Class '%s' is not exposed, skipping.", name));
classes.pop_front();
Expand Down Expand Up @@ -675,6 +682,10 @@ void DocTools::generate(bool p_basic_types) {
}
}

if (p_flags.has_flag(GENERATE_FLAG_SKIP_BASIC_TYPES)) {
return;
}

// Add a dummy Variant entry.
{
// This allows us to document the concept of Variant even though
Expand All @@ -683,11 +694,6 @@ void DocTools::generate(bool p_basic_types) {
class_list["Variant"].name = "Variant";
}

// If we don't want to populate basic types, break here.
if (!p_basic_types) {
return;
}

// Add Variant data types.
for (int i = 0; i < Variant::VARIANT_MAX; i++) {
if (i == Variant::NIL) {
Expand Down
6 changes: 5 additions & 1 deletion editor/doc_tools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ class DocTools {
void add_doc(const DocData::ClassDoc &p_class_doc);
void remove_doc(const String &p_class_name);
bool has_doc(const String &p_class_name);
void generate(bool p_basic_types = false);
enum GenerateFlags {
GENERATE_FLAG_SKIP_BASIC_TYPES = (1 << 0),
GENERATE_FLAG_EXTENSION_CLASSES_ONLY = (1 << 1),
};
void generate(BitField<GenerateFlags> p_flags = {});
Error load_classes(const String &p_dir);
Error save_classes(const String &p_default_path, const HashMap<String, String> &p_class_path, bool p_include_xml_schema = true);

Expand Down
55 changes: 25 additions & 30 deletions editor/editor_help.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2361,13 +2361,11 @@ void EditorHelp::_add_text(const String &p_bbcode) {
}

String EditorHelp::doc_version_hash;
bool EditorHelp::doc_gen_first_attempt = true;
bool EditorHelp::doc_gen_use_threads = true;
Thread EditorHelp::gen_thread;
Thread EditorHelp::worker_thread;

void EditorHelp::_wait_for_thread() {
if (gen_thread.is_started()) {
gen_thread.wait_to_finish();
if (worker_thread.is_started()) {
worker_thread.wait_to_finish();
}
}

Expand All @@ -2381,18 +2379,18 @@ String EditorHelp::get_cache_full_path() {
}

void EditorHelp::_load_doc_thread(void *p_udata) {
DEV_ASSERT(doc_gen_first_attempt);

Ref<Resource> cache_res = ResourceLoader::load(get_cache_full_path());
if (cache_res.is_valid() && cache_res->get_meta("version_hash", "") == doc_version_hash) {
Array classes = cache_res->get_meta("classes", Array());
for (int i = 0; i < classes.size(); i++) {
doc->add_doc(DocData::ClassDoc::from_dict(classes[i]));
}

// Extensions' docs are not cached. Generate them now (on the main thread).
callable_mp_static(&EditorHelp::_gen_extensions_docs).call_deferred();
} else {
// We have to go back to the main thread to start from scratch.
doc_gen_first_attempt = false;
callable_mp_static(&EditorHelp::generate_doc).bind(true).call_deferred();
// We have to go back to the main thread to start from scratch, bypassing any possibly existing cache.
callable_mp_static(&EditorHelp::generate_doc).bind(false).call_deferred();
}
}

Expand All @@ -2406,6 +2404,12 @@ void EditorHelp::_gen_doc_thread(void *p_udata) {
cache_res->set_meta("version_hash", doc_version_hash);
Array classes;
for (const KeyValue<String, DocData::ClassDoc> &E : doc->class_list) {
if (ClassDB::class_exists(E.value.name)) {
ClassDB::APIType api = ClassDB::get_api_type(E.value.name);
if (api == ClassDB::API_EXTENSION || api == ClassDB::API_EDITOR_EXTENSION) {
continue;
}
}
classes.push_back(DocData::ClassDoc::to_dict(E.value));
}
cache_res->set_meta("classes", classes);
Expand All @@ -2415,14 +2419,15 @@ void EditorHelp::_gen_doc_thread(void *p_udata) {
}
}

void EditorHelp::_gen_extensions_docs() {
Copy link
Contributor

@YuriSizov YuriSizov Nov 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be a public method (called generate_extension_docs), and I think to fully support extensions this needs to be called whenever the GDExtension manager hot reloads any library. IIUC, this right now only loads extension docs when loading the cache or generating it anew.

Though that said, we would have to also invalidate old records somehow. DocData should keep track if it's an extension class or not, if it doesn't already, and we should be able to nuke it all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In @Riteo's PR to allow GDExtensions to add fuller documentation, he's got a separate DocTools * just for the extension documentation in order to keep track of which is from extensions:

https://github.com/godotengine/godot/pull/83747/files#diff-7f898e9b24837a348e2209c5a7e5d2b686d91580787069a6bab6ea78a5d38dba

Perhaps that approach could work here?

(May not actually make sense, I don't know the documentation system that well :-))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would be necessary for this particular purpose (and besides, when we generate everything anew, we don't differentiate between sources of each class). ClassDB already has the API type stored, we can just read it and add it to the documentation entry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit late to the party but yeah, the separate DocTools is actually just a way to persist that non-generated data between regens.

doc->generate((DocTools::GENERATE_FLAG_SKIP_BASIC_TYPES | DocTools::GENERATE_FLAG_EXTENSION_CLASSES_ONLY));
}

void EditorHelp::generate_doc(bool p_use_cache) {
OS::get_singleton()->benchmark_begin_measure("EditorHelp::generate_doc");
if (doc_gen_use_threads) {
// In case not the first attempt.
_wait_for_thread();
}

DEV_ASSERT(doc_gen_first_attempt == (doc == nullptr));
// In case not the first attempt.
_wait_for_thread();

if (!doc) {
doc = memnew(DocTools);
Expand All @@ -2432,24 +2437,14 @@ void EditorHelp::generate_doc(bool p_use_cache) {
_compute_doc_version_hash();
}

if (p_use_cache && doc_gen_first_attempt && FileAccess::exists(get_cache_full_path())) {
if (doc_gen_use_threads) {
gen_thread.start(_load_doc_thread, nullptr);
} else {
_load_doc_thread(nullptr);
}
if (p_use_cache && FileAccess::exists(get_cache_full_path())) {
worker_thread.start(_load_doc_thread, nullptr);
} else {
print_verbose("Regenerating editor help cache");

// Not doable on threads unfortunately, since it instantiates all sorts of classes to get default values.
doc->generate(true);

if (doc_gen_use_threads) {
gen_thread.start(_gen_doc_thread, nullptr);
} else {
_gen_doc_thread(nullptr);
}
doc->generate();
worker_thread.start(_gen_doc_thread, nullptr);
}

OS::get_singleton()->benchmark_end_measure("EditorHelp::generate_doc");
}

Expand Down
5 changes: 2 additions & 3 deletions editor/editor_help.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,12 @@ class EditorHelp : public VBoxContainer {
void _toggle_scripts_pressed();

static String doc_version_hash;
static bool doc_gen_first_attempt;
static bool doc_gen_use_threads;
static Thread gen_thread;
static Thread worker_thread;

static void _wait_for_thread();
static void _load_doc_thread(void *p_udata);
static void _gen_doc_thread(void *p_udata);
static void _gen_extensions_docs();
static void _compute_doc_version_hash();

protected:
Expand Down
6 changes: 3 additions & 3 deletions main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2767,7 +2767,7 @@ bool Main::start() {

#ifdef TOOLS_ENABLED
String doc_tool_path;
bool doc_base = true;
BitField<DocTools::GenerateFlags> gen_flags;
String _export_preset;
bool export_debug = false;
bool export_pack_only = false;
Expand All @@ -2792,7 +2792,7 @@ bool Main::start() {
check_only = true;
#ifdef TOOLS_ENABLED
} else if (args[i] == "--no-docbase") {
doc_base = false;
gen_flags.set_flag(DocTools::GENERATE_FLAG_SKIP_BASIC_TYPES);
#ifndef DISABLE_DEPRECATED
} else if (args[i] == "--convert-3to4") {
converting_project = true;
Expand Down Expand Up @@ -2903,7 +2903,7 @@ bool Main::start() {

Error err;
DocTools doc;
doc.generate(doc_base);
doc.generate(gen_flags);

DocTools docsrc;
HashMap<String, String> doc_data_classes;
Expand Down
Loading