From fe7fbfedc018db3af943aff940909a94ccf43ffa Mon Sep 17 00:00:00 2001 From: Airtz <72342006+Airtz@users.noreply.github.com> Date: Mon, 16 Jun 2025 12:22:03 +0200 Subject: Fix typo in `error_article_table` lookup --- src/error.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/error.cpp') diff --git a/src/error.cpp b/src/error.cpp index 1492b00c7..006d5ae8d 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -823,7 +823,7 @@ gb_internal int error_value_cmp(void const *a, void const *b) { gb_global String error_article_table[][2] = { {str_lit("a "), str_lit("bit_set literal")}, {str_lit("a "), str_lit("constant declaration")}, - {str_lit("a "), str_lit("dynamiic array literal")}, + {str_lit("a "), str_lit("dynamic array literal")}, {str_lit("a "), str_lit("map index")}, {str_lit("a "), str_lit("map literal")}, {str_lit("a "), str_lit("matrix literal")}, -- cgit v1.2.3 From 1e0902677f905e752b42e2f48dcda53141b78eee Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 10 Sep 2025 17:29:11 +0100 Subject: Multithread min dep set by removing the set itself --- src/checker.cpp | 193 ++++++++++++++++++++++-------------------- src/checker.hpp | 7 +- src/entity.cpp | 1 + src/error.cpp | 16 ++-- src/llvm_backend.cpp | 17 ++-- src/llvm_backend_proc.cpp | 3 +- src/llvm_backend_stmt.cpp | 6 +- src/main.cpp | 3 +- src/name_canonicalization.cpp | 61 ++++++++++++- src/name_canonicalization.hpp | 2 + src/ptr_set.cpp | 38 +++++++++ src/threading.cpp | 38 +++++++++ src/types.cpp | 1 + 13 files changed, 263 insertions(+), 123 deletions(-) (limited to 'src/error.cpp') diff --git a/src/checker.cpp b/src/checker.cpp index 26430359c..b3a702cbd 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1712,7 +1712,7 @@ gb_internal void check_remove_expr_info(CheckerContext *c, Ast *e) { } gb_internal isize type_info_index(CheckerInfo *info, TypeInfoPair pair, bool error_on_failure) { - mutex_lock(&info->minimum_dependency_type_info_mutex); + rw_mutex_shared_lock(&info->minimum_dependency_type_info_mutex); isize entry_index = -1; u64 hash = pair.hash; @@ -1720,7 +1720,7 @@ gb_internal isize type_info_index(CheckerInfo *info, TypeInfoPair pair, bool err if (found_entry_index) { entry_index = *found_entry_index; } - mutex_unlock(&info->minimum_dependency_type_info_mutex); + rw_mutex_shared_unlock(&info->minimum_dependency_type_info_mutex); if (error_on_failure && entry_index < 0) { compiler_error("Type_Info for '%s' could not be found", type_to_string(pair.type)); @@ -2377,11 +2377,8 @@ gb_internal void add_min_dep_type_info(Checker *c, Type *t) { return; } - { - MUTEX_GUARD(&c->info.minimum_dependency_type_info_mutex); - if (type_set_update(&c->info.min_dep_type_info_set, t)) { - return; - } + if (type_set_update_with_mutex(&c->info.min_dep_type_info_set, t, &c->info.min_dep_type_info_set_mutex)) { + return; } // Add nested types @@ -2555,13 +2552,15 @@ gb_internal void add_min_dep_type_info(Checker *c, Type *t) { } } +gb_internal void add_dependency_to_set_threaded(Checker *c, Entity *entity); + +gb_global std::atomic global_checker_ptr; + gb_internal void add_dependency_to_set(Checker *c, Entity *entity) { if (entity == nullptr) { return; } - CheckerInfo *info = &c->info; - if (entity->type != nullptr && is_type_polymorphic(entity->type)) { DeclInfo *decl = decl_info_of_entity(entity); @@ -2570,11 +2569,8 @@ gb_internal void add_dependency_to_set(Checker *c, Entity *entity) { } } - { - MUTEX_GUARD(&info->minimum_dependency_type_info_mutex); - if (ptr_set_update(&info->minimum_dependency_set, entity)) { - return; - } + if (entity->min_dep_count.fetch_add(1, std::memory_order_relaxed) > 0) { + return; } DeclInfo *decl = decl_info_of_entity(entity); @@ -2584,46 +2580,45 @@ gb_internal void add_dependency_to_set(Checker *c, Entity *entity) { for (TypeInfoPair const tt : decl->type_info_deps) { add_min_dep_type_info(c, tt.type); } - for (Entity *e : decl->deps) { - add_dependency_to_set(c, e); - if (e->kind == Entity_Procedure && e->Procedure.is_foreign) { - Entity *fl = e->Procedure.foreign_library; - if (fl != nullptr) { - GB_ASSERT_MSG(fl->kind == Entity_LibraryName && - (fl->flags&EntityFlag_Used), - "%.*s", LIT(entity->token.string)); - add_dependency_to_set(c, fl); + switch (e->kind) { + case Entity_Procedure: + if (e->Procedure.is_foreign) { + Entity *fl = e->Procedure.foreign_library; + if (fl != nullptr) { + GB_ASSERT_MSG(fl->kind == Entity_LibraryName && + (fl->flags&EntityFlag_Used), + "%.*s", LIT(entity->token.string)); + add_dependency_to_set(c, fl); + } } - } else if (e->kind == Entity_Variable && e->Variable.is_foreign) { - Entity *fl = e->Variable.foreign_library; - if (fl != nullptr) { - GB_ASSERT_MSG(fl->kind == Entity_LibraryName && - (fl->flags&EntityFlag_Used), - "%.*s", LIT(entity->token.string)); - add_dependency_to_set(c, fl); + break; + case Entity_Variable: + if (e->Variable.is_foreign) { + Entity *fl = e->Variable.foreign_library; + if (fl != nullptr) { + GB_ASSERT_MSG(fl->kind == Entity_LibraryName && + (fl->flags&EntityFlag_Used), + "%.*s", LIT(entity->token.string)); + add_dependency_to_set(c, fl); + } } + break; } } -} -struct AddDependecyToSetWorkerData { - Checker *c; - Entity *entity; -}; - -gb_internal void add_dependency_to_set_threaded(Checker *c, Entity *entity); + for (Entity *e : decl->deps) { + add_dependency_to_set(c, e); + } +} gb_internal WORKER_TASK_PROC(add_dependency_to_set_worker) { - AddDependecyToSetWorkerData *wd = cast(AddDependecyToSetWorkerData *)data; - Checker *c = wd->c; - Entity *entity = wd->entity; + Checker *c = global_checker_ptr.load(std::memory_order_relaxed); + Entity *entity = cast(Entity *)data; if (entity == nullptr) { return 0; } - CheckerInfo *info = &c->info; - if (entity->type != nullptr && is_type_polymorphic(entity->type)) { DeclInfo *decl = decl_info_of_entity(entity); @@ -2632,11 +2627,8 @@ gb_internal WORKER_TASK_PROC(add_dependency_to_set_worker) { } } - { - MUTEX_GUARD(&info->minimum_dependency_type_info_mutex); - if (ptr_set_update(&info->minimum_dependency_set, entity)) { - return 0; - } + if (entity->min_dep_count.fetch_add(1, std::memory_order_relaxed) > 0) { + return 0; } DeclInfo *decl = decl_info_of_entity(entity); @@ -2648,25 +2640,36 @@ gb_internal WORKER_TASK_PROC(add_dependency_to_set_worker) { } for (Entity *e : decl->deps) { - add_dependency_to_set(c, e); - if (e->kind == Entity_Procedure && e->Procedure.is_foreign) { - Entity *fl = e->Procedure.foreign_library; - if (fl != nullptr) { - GB_ASSERT_MSG(fl->kind == Entity_LibraryName && - (fl->flags&EntityFlag_Used), - "%.*s", LIT(entity->token.string)); - add_dependency_to_set_threaded(c, fl); + switch (e->kind) { + case Entity_Procedure: + if (e->Procedure.is_foreign) { + Entity *fl = e->Procedure.foreign_library; + if (fl != nullptr) { + GB_ASSERT_MSG(fl->kind == Entity_LibraryName && + (fl->flags&EntityFlag_Used), + "%.*s", LIT(entity->token.string)); + add_dependency_to_set_threaded(c, fl); + } } - } else if (e->kind == Entity_Variable && e->Variable.is_foreign) { - Entity *fl = e->Variable.foreign_library; - if (fl != nullptr) { - GB_ASSERT_MSG(fl->kind == Entity_LibraryName && - (fl->flags&EntityFlag_Used), - "%.*s", LIT(entity->token.string)); - add_dependency_to_set_threaded(c, fl); + break; + case Entity_Variable: + if (e->Variable.is_foreign) { + Entity *fl = e->Variable.foreign_library; + if (fl != nullptr) { + GB_ASSERT_MSG(fl->kind == Entity_LibraryName && + (fl->flags&EntityFlag_Used), + "%.*s", LIT(entity->token.string)); + add_dependency_to_set_threaded(c, fl); + } } + break; } } + + for (Entity *e : decl->deps) { + add_dependency_to_set_threaded(c, e); + } + return 0; } @@ -2675,11 +2678,7 @@ gb_internal void add_dependency_to_set_threaded(Checker *c, Entity *entity) { if (entity == nullptr) { return; } - - AddDependecyToSetWorkerData *wd = gb_alloc_item(temporary_allocator(), AddDependecyToSetWorkerData); - wd->c = c; - wd->entity = entity; - thread_pool_add_task(add_dependency_to_set_worker, wd); + thread_pool_add_task(add_dependency_to_set_worker, entity); } @@ -2732,27 +2731,35 @@ gb_internal void collect_testing_procedures_of_package(Checker *c, AstPackage *p } gb_internal void generate_minimum_dependency_set_internal(Checker *c, Entity *start) { + // auto const &add_to_set = add_dependency_to_set; + auto const &add_to_set = add_dependency_to_set_threaded; + + Scope *builtin_scope = builtin_pkg->scope; for_array(i, c->info.definitions) { Entity *e = c->info.definitions[i]; - if (e->scope == builtin_pkg->scope) { + if (e->scope == builtin_scope) { if (e->type == nullptr) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); + } + } else if (e->kind == Entity_Procedure) { + if (e->Procedure.is_export) { + add_to_set(c, e); + } + } else if (e->kind == Entity_Variable) { + if (e->Variable.is_export) { + add_to_set(c, e); } - } else if (e->kind == Entity_Procedure && e->Procedure.is_export) { - add_dependency_to_set_threaded(c, e); - } else if (e->kind == Entity_Variable && e->Variable.is_export) { - add_dependency_to_set_threaded(c, e); } } for (Entity *e; mpsc_dequeue(&c->info.required_foreign_imports_through_force_queue, &e); /**/) { array_add(&c->info.required_foreign_imports_through_force, e); - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } for (Entity *e; mpsc_dequeue(&c->info.required_global_variable_queue, &e); /**/) { e->flags |= EntityFlag_Used; - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } for_array(i, c->info.entities) { @@ -2760,16 +2767,16 @@ gb_internal void generate_minimum_dependency_set_internal(Checker *c, Entity *st switch (e->kind) { case Entity_Variable: if (e->Variable.is_export) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } else if (e->flags & EntityFlag_Require) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } break; case Entity_Procedure: if (e->Procedure.is_export) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } else if (e->flags & EntityFlag_Require) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } if (e->flags & EntityFlag_Init) { Type *t = base_type(e->type); @@ -2809,7 +2816,7 @@ gb_internal void generate_minimum_dependency_set_internal(Checker *c, Entity *st if (is_init) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); array_add(&c->info.init_procedures, e); } } else if (e->flags & EntityFlag_Fini) { @@ -2844,7 +2851,7 @@ gb_internal void generate_minimum_dependency_set_internal(Checker *c, Entity *st } if (is_fini) { - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); array_add(&c->info.fini_procedures, e); } } @@ -2861,7 +2868,7 @@ gb_internal void generate_minimum_dependency_set_internal(Checker *c, Entity *st Entity *e = entry.value; if (e != nullptr) { e->flags |= EntityFlag_Used; - add_dependency_to_set_threaded(c, e); + add_to_set(c, e); } } @@ -2876,16 +2883,11 @@ gb_internal void generate_minimum_dependency_set_internal(Checker *c, Entity *st } } else if (start != nullptr) { start->flags |= EntityFlag_Used; - add_dependency_to_set_threaded(c, start); + add_to_set(c, start); } } gb_internal void generate_minimum_dependency_set(Checker *c, Entity *start) { - isize entity_count = c->info.entities.count; - isize min_dep_set_cap = next_pow2_isize(entity_count*4); // empirically determined factor - - ptr_set_init(&c->info.minimum_dependency_set, min_dep_set_cap); - #define FORCE_ADD_RUNTIME_ENTITIES(condition, ...) do { \ if (condition) { \ String entities[] = {__VA_ARGS__}; \ @@ -6267,8 +6269,10 @@ gb_internal void check_unchecked_bodies(Checker *c) { // use the `procs_to_check` array global_procedure_body_in_worker_queue = false; - for (Entity *e : c->info.minimum_dependency_set) { - check_procedure_later_from_entity(c, e, "check_unchecked_bodies"); + for (Entity *e : c->info.entities) { + if (e->min_dep_count.load(std::memory_order_relaxed) > 0) { + check_procedure_later_from_entity(c, e, "check_unchecked_bodies"); + } } if (!global_procedure_body_in_worker_queue) { @@ -7042,6 +7046,7 @@ gb_internal void check_merge_queues_into_arrays(Checker *c) { } check_add_entities_from_queues(c); check_add_definitions_from_queues(c); + thread_pool_wait(); } gb_internal GB_COMPARE_PROC(init_procedures_cmp) { @@ -7100,7 +7105,7 @@ gb_internal void add_type_info_for_type_definitions(Checker *c) { Entity *e = c->info.definitions[i]; if (e->kind == Entity_TypeName && e->type != nullptr && is_type_typed(e->type)) { i64 align = type_align_of(e->type); - if (align > 0 && ptr_set_exists(&c->info.minimum_dependency_set, e)) { + if (align > 0 && e->min_dep_count.load(std::memory_order_relaxed) > 0) { add_type_info_type(&c->builtin_ctx, e->type); } } @@ -7170,6 +7175,8 @@ gb_internal void check_update_dependency_tree_for_procedures(Checker *c) { gb_internal void check_parsed_files(Checker *c) { + global_checker_ptr.store(c, std::memory_order_relaxed); + TIME_SECTION("map full filepaths to scope"); add_type_info_type(&c->builtin_ctx, t_invalid); @@ -7312,11 +7319,9 @@ gb_internal void check_parsed_files(Checker *c) { check_unchecked_bodies(c); TIME_SECTION("check #soa types"); - check_merge_queues_into_arrays(c); - thread_pool_wait(); - TIME_SECTION("update minimum dependency set"); + TIME_SECTION("update minimum dependency set again"); generate_minimum_dependency_set_internal(c, c->info.entry_point); // NOTE(laytan): has to be ran after generate_minimum_dependency_set, diff --git a/src/checker.hpp b/src/checker.hpp index 1da46b74a..8b4d61ee2 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -449,11 +449,10 @@ struct CheckerInfo { Scope * init_scope; Entity * entry_point; - BlockingMutex minimum_dependency_set_mutex; - PtrSet minimum_dependency_set; - - BlockingMutex minimum_dependency_type_info_mutex; + RwMutex minimum_dependency_type_info_mutex; PtrMap min_dep_type_info_index_map; + + RWSpinLock min_dep_type_info_set_mutex; TypeSet min_dep_type_info_set; Array type_info_types_hash_map; // 2 * type_info_types.count diff --git a/src/entity.cpp b/src/entity.cpp index 6c0aa6ace..5ca3fa916 100644 --- a/src/entity.cpp +++ b/src/entity.cpp @@ -164,6 +164,7 @@ struct Entity { u64 id; std::atomic flags; std::atomic state; + std::atomic min_dep_count; Token token; Scope * scope; Type * type; diff --git a/src/error.cpp b/src/error.cpp index 006d5ae8d..10bf1caf5 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -86,7 +86,7 @@ gb_internal char *token_pos_to_string(TokenPos const &pos); gb_internal bool set_file_path_string(i32 index, String const &path) { bool ok = false; GB_ASSERT(index >= 0); - mutex_lock(&global_error_collector.path_mutex); + // mutex_lock(&global_error_collector.path_mutex); mutex_lock(&global_files_mutex); if (index >= global_file_path_strings.count) { @@ -99,14 +99,14 @@ gb_internal bool set_file_path_string(i32 index, String const &path) { } mutex_unlock(&global_files_mutex); - mutex_unlock(&global_error_collector.path_mutex); + // mutex_unlock(&global_error_collector.path_mutex); return ok; } gb_internal bool thread_safe_set_ast_file_from_id(i32 index, AstFile *file) { bool ok = false; GB_ASSERT(index >= 0); - mutex_lock(&global_error_collector.path_mutex); + // mutex_lock(&global_error_collector.path_mutex); mutex_lock(&global_files_mutex); if (index >= global_files.count) { @@ -118,13 +118,13 @@ gb_internal bool thread_safe_set_ast_file_from_id(i32 index, AstFile *file) { ok = true; } mutex_unlock(&global_files_mutex); - mutex_unlock(&global_error_collector.path_mutex); + // mutex_unlock(&global_error_collector.path_mutex); return ok; } gb_internal String get_file_path_string(i32 index) { GB_ASSERT(index >= 0); - mutex_lock(&global_error_collector.path_mutex); + // mutex_lock(&global_error_collector.path_mutex); mutex_lock(&global_files_mutex); String path = {}; @@ -133,13 +133,13 @@ gb_internal String get_file_path_string(i32 index) { } mutex_unlock(&global_files_mutex); - mutex_unlock(&global_error_collector.path_mutex); + // mutex_unlock(&global_error_collector.path_mutex); return path; } gb_internal AstFile *thread_safe_get_ast_file_from_id(i32 index) { GB_ASSERT(index >= 0); - mutex_lock(&global_error_collector.path_mutex); + // mutex_lock(&global_error_collector.path_mutex); mutex_lock(&global_files_mutex); AstFile *file = nullptr; @@ -148,7 +148,7 @@ gb_internal AstFile *thread_safe_get_ast_file_from_id(i32 index) { } mutex_unlock(&global_files_mutex); - mutex_unlock(&global_error_collector.path_mutex); + // mutex_unlock(&global_error_collector.path_mutex); return file; } diff --git a/src/llvm_backend.cpp b/src/llvm_backend.cpp index ff17e9c10..11b979774 100644 --- a/src/llvm_backend.cpp +++ b/src/llvm_backend.cpp @@ -2097,8 +2097,6 @@ gb_internal GB_COMPARE_PROC(llvm_global_entity_cmp) { } gb_internal void lb_create_global_procedures_and_types(lbGenerator *gen, CheckerInfo *info, bool do_threading) { - auto *min_dep_set = &info->minimum_dependency_set; - for (Entity *e : info->entities) { String name = e->token.string; Scope * scope = e->scope; @@ -2135,11 +2133,16 @@ gb_internal void lb_create_global_procedures_and_types(lbGenerator *gen, Checker } } - if (!polymorphic_struct && !ptr_set_exists(min_dep_set, e)) { + if (!polymorphic_struct && e->min_dep_count.load(std::memory_order_relaxed) == 0) { // NOTE(bill): Nothing depends upon it so doesn't need to be built continue; } + // if (!polymorphic_struct && !ptr_set_exists(min_dep_set, e)) { + // // NOTE(bill): Nothing depends upon it so doesn't need to be built + // continue; + // } + lbModule *m = &gen->default_module; if (USE_SEPARATE_MODULES) { m = lb_module_of_entity(gen, e); @@ -2845,8 +2848,6 @@ gb_internal bool lb_generate_code(lbGenerator *gen) { lbModule *default_module = &gen->default_module; CheckerInfo *info = gen->info; - auto *min_dep_set = &info->minimum_dependency_set; - switch (build_context.metrics.arch) { case TargetArch_amd64: case TargetArch_i386: @@ -3184,10 +3185,14 @@ gb_internal bool lb_generate_code(lbGenerator *gen) { continue; } - if (!ptr_set_exists(min_dep_set, e)) { + if (e->min_dep_count.load(std::memory_order_relaxed) == 0) { continue; } + // if (!ptr_set_exists(min_dep_set, e)) { + // continue; + // } + DeclInfo *decl = decl_info_of_entity(e); if (decl == nullptr) { continue; diff --git a/src/llvm_backend_proc.cpp b/src/llvm_backend_proc.cpp index f2e6662c8..06829efab 100644 --- a/src/llvm_backend_proc.cpp +++ b/src/llvm_backend_proc.cpp @@ -798,9 +798,8 @@ gb_internal void lb_end_procedure_body(lbProcedure *p) { gb_internal void lb_build_nested_proc(lbProcedure *p, AstProcLit *pd, Entity *e) { GB_ASSERT(pd->body != nullptr); lbModule *m = p->module; - auto *min_dep_set = &m->info->minimum_dependency_set; - if (ptr_set_exists(min_dep_set, e) == false) { + if (e->min_dep_count.load(std::memory_order_relaxed) == 0) { // NOTE(bill): Nothing depends upon it so doesn't need to be built return; } diff --git a/src/llvm_backend_stmt.cpp b/src/llvm_backend_stmt.cpp index 5481ca447..590920b59 100644 --- a/src/llvm_backend_stmt.cpp +++ b/src/llvm_backend_stmt.cpp @@ -3,8 +3,6 @@ gb_internal void lb_build_constant_value_decl(lbProcedure *p, AstValueDecl *vd) return; } - auto *min_dep_set = &p->module->info->minimum_dependency_set; - for (Ast *ident : vd->names) { GB_ASSERT(ident->kind == Ast_Ident); Entity *e = entity_of_node(ident); @@ -21,7 +19,7 @@ gb_internal void lb_build_constant_value_decl(lbProcedure *p, AstValueDecl *vd) } } - if (!polymorphic_struct && !ptr_set_exists(min_dep_set, e)) { + if (!polymorphic_struct && e->min_dep_count.load(std::memory_order_relaxed) == 0) { continue; } @@ -56,7 +54,7 @@ gb_internal void lb_build_constant_value_decl(lbProcedure *p, AstValueDecl *vd) if (gpd) { rw_mutex_shared_lock(&gpd->mutex); for (Entity *e : gpd->procs) { - if (!ptr_set_exists(min_dep_set, e)) { + if (e->min_dep_count.load(std::memory_order_relaxed) == 0) { continue; } DeclInfo *d = decl_info_of_entity(e); diff --git a/src/main.cpp b/src/main.cpp index db4dee080..184b1eaac 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3050,7 +3050,8 @@ gb_internal void print_show_unused(Checker *c) { if (e->token.string == "_") { continue; } - if (ptr_set_exists(&info->minimum_dependency_set, e)) { + + if (e->min_dep_count.load(std::memory_order_relaxed) > 0) { continue; } array_add(&unused, e); diff --git a/src/name_canonicalization.cpp b/src/name_canonicalization.cpp index 6a4538e26..8bacfabc6 100644 --- a/src/name_canonicalization.cpp +++ b/src/name_canonicalization.cpp @@ -57,10 +57,13 @@ gb_internal isize type_set__find(TypeSet *s, TypeInfoPair pair) { usize mask = s->capacity-1; usize hash_index = cast(usize)hash & mask; for (usize i = 0; i < s->capacity; i++) { - Type *key = s->keys[hash_index].type; - if (are_types_identical_unique_tuples(key, pair.type)) { + auto *e = &s->keys[hash_index]; + u64 hash = e->hash; + Type *key = e->type; + if (hash == pair.hash && + are_types_identical_unique_tuples(key, pair.type)) { return hash_index; - } else if (key == 0) { + } else if (key == nullptr) { return -1; } hash_index = (hash_index+1)&mask; @@ -164,6 +167,48 @@ gb_internal bool type_set_update(TypeSet *s, Type *ptr) { // returns true if it return type_set_update(s, pair); } +gb_internal bool type_set_update_with_mutex(TypeSet *s, TypeInfoPair pair, RWSpinLock *m) { // returns true if it previously existsed + rwlock_acquire_upgrade(m); + if (type_set_exists(s, pair)) { + rwlock_release_upgrade(m); + return true; + } + + rwlock_release_upgrade_and_acquire_write(m); + defer (rwlock_release_write(m)); + + if (s->keys == nullptr) { + type_set_init(s); + } else if (type_set__full(s)) { + type_set_grow(s); + } + GB_ASSERT(s->count < s->capacity); + GB_ASSERT(s->capacity >= 0); + + usize mask = s->capacity-1; + usize hash = cast(usize)pair.hash; + usize hash_index = (cast(usize)hash) & mask; + GB_ASSERT(hash_index < s->capacity); + for (usize i = 0; i < s->capacity; i++) { + TypeInfoPair *key = &s->keys[hash_index]; + GB_ASSERT(!are_types_identical_unique_tuples(key->type, pair.type)); + if (key->hash == TYPE_SET_TOMBSTONE || key->hash == 0) { + *key = pair; + s->count++; + return false; + } + hash_index = (hash_index+1)&mask; + } + + GB_PANIC("ptr set out of memory"); + return false; +} + +gb_internal bool type_set_update_with_mutex(TypeSet *s, Type *ptr, RWSpinLock *m) { // returns true if it previously existsed + TypeInfoPair pair = {ptr, type_hash_canonical_type(ptr)}; + return type_set_update_with_mutex(s, pair, m); +} + gb_internal Type *type_set_add(TypeSet *s, Type *ptr) { type_set_update(s, ptr); @@ -328,12 +373,20 @@ gb_internal u64 type_hash_canonical_type(Type *type) { if (type == nullptr) { return 0; } + u64 prev_hash = type->canonical_hash.load(std::memory_order_relaxed); + if (prev_hash != 0) { + return prev_hash; + } + u64 hash = fnv64a(nullptr, 0); TypeWriter w = {}; type_writer_make_hasher(&w, &hash); write_type_to_canonical_string(&w, type); + hash = hash ? hash : 1; + + type->canonical_hash.store(hash, std::memory_order_relaxed); - return hash ? hash : 1; + return hash; } gb_internal String type_to_canonical_string(gbAllocator allocator, Type *type) { diff --git a/src/name_canonicalization.hpp b/src/name_canonicalization.hpp index 304aff42e..00b450fbe 100644 --- a/src/name_canonicalization.hpp +++ b/src/name_canonicalization.hpp @@ -102,6 +102,8 @@ gb_internal Type *type_set_add (TypeSet *s, Type *ptr); gb_internal Type *type_set_add (TypeSet *s, TypeInfoPair pair); gb_internal bool type_set_update (TypeSet *s, Type *ptr); // returns true if it previously existed gb_internal bool type_set_update (TypeSet *s, TypeInfoPair pair); // returns true if it previously existed +gb_internal bool type_set_update_with_mutex(TypeSet *s, TypeInfoPair pair, RWSpinLock *m); +gb_internal bool type_set_update_with_mutex(TypeSet *s, Type *ptr, RWSpinLock *m); gb_internal bool type_set_exists (TypeSet *s, Type *ptr); gb_internal void type_set_remove (TypeSet *s, Type *ptr); gb_internal void type_set_clear (TypeSet *s); diff --git a/src/ptr_set.cpp b/src/ptr_set.cpp index 5097e2bb6..06c1e4a58 100644 --- a/src/ptr_set.cpp +++ b/src/ptr_set.cpp @@ -134,6 +134,44 @@ gb_internal bool ptr_set_update(PtrSet *s, T ptr) { // returns true if it pre return false; } +template +gb_internal bool ptr_set_update_with_mutex(PtrSet *s, T ptr, RWSpinLock *m) { // returns true if it previously existsed + rwlock_acquire_upgrade(m); + if (ptr_set_exists(s, ptr)) { + rwlock_release_upgrade(m); + return true; + } + + rwlock_release_upgrade_and_acquire_write(m); + defer (rwlock_release_write(m)); + + if (s->keys == nullptr) { + ptr_set_init(s); + } else if (ptr_set__full(s)) { + ptr_set_grow(s); + } + GB_ASSERT(s->count < s->capacity); + GB_ASSERT(s->capacity >= 0); + + usize mask = s->capacity-1; + u32 hash = ptr_map_hash_key(ptr); + usize hash_index = (cast(usize)hash) & mask; + GB_ASSERT(hash_index < s->capacity); + for (usize i = 0; i < s->capacity; i++) { + T *key = &s->keys[hash_index]; + GB_ASSERT(*key != ptr); + if (*key == (T)PtrSet::TOMBSTONE || *key == 0) { + *key = ptr; + s->count++; + return false; + } + hash_index = (hash_index+1)&mask; + } + + GB_PANIC("ptr set out of memory"); + return false; +} + template gb_internal T ptr_set_add(PtrSet *s, T ptr) { ptr_set_update(s, ptr); diff --git a/src/threading.cpp b/src/threading.cpp index a0d1c4049..f1d9264e3 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -448,6 +448,44 @@ gb_internal void semaphore_wait(Semaphore *s) { } #endif +static const int RWLOCK_WRITER = 1; +static const int RWLOCK_UPGRADED = 2; +static const int RWLOCK_READER = 4; +struct RWSpinLock { + Futex bits; +}; + +void rwlock_release_write(RWSpinLock *l) { + l->bits.fetch_and(~(RWLOCK_WRITER | RWLOCK_UPGRADED), std::memory_order_release); + futex_signal(&l->bits); +} + +bool rwlock_try_acquire_upgrade(RWSpinLock *l) { + int value = l->bits.fetch_or(RWLOCK_UPGRADED, std::memory_order_acquire); + return (value & (RWLOCK_UPGRADED | RWLOCK_WRITER)) == 0; +} + +void rwlock_acquire_upgrade(RWSpinLock *l) { + while (!rwlock_try_acquire_upgrade(l)) { + futex_wait(&l->bits, RWLOCK_UPGRADED); + } +} +void rwlock_release_upgrade(RWSpinLock *l) { + l->bits.fetch_add(-RWLOCK_UPGRADED, std::memory_order_acq_rel); +} + +bool rwlock_try_release_upgrade_and_acquire_write(RWSpinLock *l) { + int expect = RWLOCK_UPGRADED; + return l->bits.compare_exchange_strong(expect, RWLOCK_WRITER, std::memory_order_acq_rel); +} + +void rwlock_release_upgrade_and_acquire_write(RWSpinLock *l) { + while (!rwlock_try_release_upgrade_and_acquire_write(l)) { + futex_wait(&l->bits, RWLOCK_WRITER); + } +} + + struct Parker { Futex state; }; diff --git a/src/types.cpp b/src/types.cpp index 6b94b1690..44f9394c7 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -334,6 +334,7 @@ struct Type { // NOTE(bill): These need to be at the end to not affect the unionized data std::atomic cached_size; std::atomic cached_align; + std::atomic canonical_hash; std::atomic flags; // TypeFlag bool failure; }; -- cgit v1.2.3 From a36a8722dc823c6fe143f7935e79467c6569bc00 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 10 Sep 2025 19:30:32 +0100 Subject: Minimize more thread contention --- src/build_settings.cpp | 2 +- src/check_decl.cpp | 4 ++-- src/check_expr.cpp | 16 ++++++-------- src/check_stmt.cpp | 5 +++-- src/check_type.cpp | 9 ++++---- src/checker.cpp | 58 +++++++++++++++++++++++++++----------------------- src/checker.hpp | 15 +++++++------ src/entity.cpp | 2 +- src/error.cpp | 11 ++++++++++ src/threading.cpp | 10 ++++----- 10 files changed, 74 insertions(+), 58 deletions(-) (limited to 'src/error.cpp') diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 0081fabee..0c88f3d13 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -1163,7 +1163,7 @@ gb_internal String internal_odin_root_dir(void) { return global_module_path; } - auto path_buf = array_make(heap_allocator(), 300); + auto path_buf = array_make(temporary_allocator(), 300); defer (array_free(&path_buf)); len = 0; diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 7dd9db105..49731ad60 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -1981,9 +1981,9 @@ gb_internal bool check_proc_body(CheckerContext *ctx_, Token token, DeclInfo *de ast_node(bs, BlockStmt, body); + TEMPORARY_ALLOCATOR_GUARD(); Array using_entities = {}; - using_entities.allocator = heap_allocator(); - defer (array_free(&using_entities)); + using_entities.allocator = temporary_allocator(); { if (type->Proc.param_count > 0) { diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 84f1c6f0a..bdbccb4f8 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -7516,11 +7516,10 @@ gb_internal CallArgumentData check_call_arguments(CheckerContext *c, Operand *op return check_call_arguments_proc_group(c, operand, call); } - auto positional_operands = array_make(heap_allocator(), 0, positional_args.count); - auto named_operands = array_make(heap_allocator(), 0, 0); + TEMPORARY_ALLOCATOR_GUARD(); - defer (array_free(&positional_operands)); - defer (array_free(&named_operands)); + auto positional_operands = array_make(temporary_allocator(), 0, positional_args.count); + auto named_operands = array_make(temporary_allocator(), 0, 0); if (positional_args.count > 0) { Entity **lhs = nullptr; @@ -7623,11 +7622,10 @@ gb_internal CallArgumentError check_polymorphic_record_type(CheckerContext *c, O { // NOTE(bill, 2019-10-26): Allow a cycle in the parameters but not in the fields themselves auto prev_type_path = c->type_path; - c->type_path = new_checker_type_path(); - defer ({ - destroy_checker_type_path(c->type_path); - c->type_path = prev_type_path; - }); + TEMPORARY_ALLOCATOR_GUARD(); + + c->type_path = new_checker_type_path(temporary_allocator()); + defer (c->type_path = prev_type_path); if (is_call_expr_field_value(ce)) { named_fields = true; diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index e03d4a7ae..ba9c08fed 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2567,8 +2567,9 @@ gb_internal void check_return_stmt(CheckerContext *ctx, Ast *node) { result_count = proc_type->Proc.results->Tuple.variables.count; } - auto operands = array_make(heap_allocator(), 0, 2*rs->results.count); - defer (array_free(&operands)); + TEMPORARY_ALLOCATOR_GUARD(); + + auto operands = array_make(temporary_allocator(), 0, 2*rs->results.count); check_unpack_arguments(ctx, result_entities, result_count, &operands, rs->results, UnpackFlag_AllowOk); diff --git a/src/check_type.cpp b/src/check_type.cpp index 4c995588f..e99909d6b 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -3512,8 +3512,9 @@ gb_internal bool check_type_internal(CheckerContext *ctx, Ast *e, Type **type, T case_ast_node(pt, PointerType, e); CheckerContext c = *ctx; - c.type_path = new_checker_type_path(); - defer (destroy_checker_type_path(c.type_path)); + + TEMPORARY_ALLOCATOR_GUARD(); + c.type_path = new_checker_type_path(temporary_allocator()); Type *elem = t_invalid; Operand o = {}; @@ -3747,8 +3748,8 @@ gb_internal bool check_type_internal(CheckerContext *ctx, Ast *e, Type **type, T gb_internal Type *check_type(CheckerContext *ctx, Ast *e) { CheckerContext c = *ctx; - c.type_path = new_checker_type_path(); - defer (destroy_checker_type_path(c.type_path)); + TEMPORARY_ALLOCATOR_GUARD(); + c.type_path = new_checker_type_path(temporary_allocator()); return check_type_expr(&c, e, nullptr); } diff --git a/src/checker.cpp b/src/checker.cpp index 04e46d0e6..c1d6302ad 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1441,7 +1441,8 @@ gb_internal void init_checker_info(CheckerInfo *i) { map_init(&i->objc_method_implementations); string_map_init(&i->load_file_cache); - array_init(&i->all_procedures, heap_allocator()); + array_init(&i->all_procedures, a); + mpsc_init(&i->all_procedures_queue, a); mpsc_init(&i->entity_queue, a); // 1<<20); mpsc_init(&i->definition_queue, a); //); // 1<<20); @@ -1475,6 +1476,10 @@ gb_internal void destroy_checker_info(CheckerInfo *i) { array_free(&i->required_foreign_imports_through_force); array_free(&i->defineables); + array_free(&i->all_procedures); + + mpsc_destroy(&i->all_procedures_queue); + mpsc_destroy(&i->entity_queue); mpsc_destroy(&i->definition_queue); mpsc_destroy(&i->required_global_variable_queue); @@ -1502,12 +1507,12 @@ gb_internal CheckerContext make_checker_context(Checker *c) { ctx.scope = builtin_pkg->scope; ctx.pkg = builtin_pkg; - ctx.type_path = new_checker_type_path(); + ctx.type_path = new_checker_type_path(heap_allocator()); ctx.type_level = 0; return ctx; } gb_internal void destroy_checker_context(CheckerContext *ctx) { - destroy_checker_type_path(ctx->type_path); + destroy_checker_type_path(ctx->type_path, heap_allocator()); } gb_internal bool add_curr_ast_file(CheckerContext *ctx, AstFile *file) { @@ -1758,7 +1763,7 @@ gb_internal void add_untyped(CheckerContext *c, Ast *expr, AddressingMode mode, check_set_expr_info(c, expr, mode, type, value); } -gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMode mode, Type *type, ExactValue const &value) { +gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMode mode, Type *type, ExactValue const &value, bool use_mutex) { if (expr == nullptr) { return; } @@ -1776,7 +1781,7 @@ gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMo mutex = &ctx->pkg->type_and_value_mutex; } - mutex_lock(mutex); + if (use_mutex) mutex_lock(mutex); Ast *prev_expr = nullptr; while (prev_expr != expr) { prev_expr = expr; @@ -1801,7 +1806,7 @@ gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMo break; }; } - mutex_unlock(mutex); + if (use_mutex) mutex_unlock(mutex); } gb_internal void add_entity_definition(CheckerInfo *i, Ast *identifier, Entity *entity) { @@ -2345,11 +2350,9 @@ gb_internal void check_procedure_later(Checker *c, ProcInfo *info) { } if (DEBUG_CHECK_ALL_PROCEDURES) { - MUTEX_GUARD_BLOCK(&c->info.all_procedures_mutex) { - GB_ASSERT(info != nullptr); - GB_ASSERT(info->decl != nullptr); - array_add(&c->info.all_procedures, info); - } + GB_ASSERT(info != nullptr); + GB_ASSERT(info->decl != nullptr); + mpsc_enqueue(&c->info.all_procedures_queue, info); } } @@ -3195,19 +3198,17 @@ gb_internal Type *find_type_in_pkg(CheckerInfo *info, String const &pkg, String return e->type; } -gb_internal CheckerTypePath *new_checker_type_path() { - gbAllocator a = heap_allocator(); - auto *tp = gb_alloc_item(a, CheckerTypePath); - array_init(tp, a, 0, 16); +gb_internal CheckerTypePath *new_checker_type_path(gbAllocator allocator) { + auto *tp = gb_alloc_item(allocator, CheckerTypePath); + array_init(tp, allocator, 0, 16); return tp; } -gb_internal void destroy_checker_type_path(CheckerTypePath *tp) { +gb_internal void destroy_checker_type_path(CheckerTypePath *tp, gbAllocator allocator) { array_free(tp); - gb_free(heap_allocator(), tp); + gb_free(allocator, tp); } - gb_internal void check_type_path_push(CheckerContext *c, Entity *e) { GB_ASSERT(c->type_path != nullptr); GB_ASSERT(e != nullptr); @@ -5283,9 +5284,10 @@ gb_internal void check_add_import_decl(CheckerContext *ctx, Ast *decl) { GB_ASSERT(scope->flags&ScopeFlag_Pkg); - if (ptr_set_update(&parent_scope->imported, scope)) { - // error(token, "Multiple import of the same file within this scope"); - } + ptr_set_add(&parent_scope->imported, scope); + // if (ptr_set_update(&parent_scope->imported, scope)) { + // // error(token, "Multiple import of the same file within this scope"); + // } String import_name = path_to_entity_name(id->import_name.string, id->fullpath, false); if (is_blank_ident(import_name)) { @@ -6041,10 +6043,10 @@ gb_internal void calculate_global_init_order(Checker *c) { CheckerInfo *info = &c->info; TIME_SECTION("calculate_global_init_order: generate entity dependency graph"); - Arena *arena = get_arena(ThreadArena_Temporary); - ArenaTempGuard arena_guard(arena); + Arena *temporary_arena = get_arena(ThreadArena_Temporary); + ArenaTempGuard temporary_arena_guard(temporary_arena); - Array dep_graph = generate_entity_dependency_graph(info, arena); + Array dep_graph = generate_entity_dependency_graph(info, temporary_arena); TIME_SECTION("calculate_global_init_order: priority queue create"); // NOTE(bill): Priority queue @@ -6321,8 +6323,9 @@ gb_internal void check_safety_all_procedures_for_unchecked(Checker *c) { defer (map_destroy(&untyped)); - for_array(i, c->info.all_procedures) { - ProcInfo *pi = c->info.all_procedures[i]; + array_reserve(&c->info.all_procedures, c->info.all_procedures_queue.count.load()); + + for (ProcInfo *pi = nullptr; mpsc_dequeue(&c->info.all_procedures_queue, &pi); /**/) { GB_ASSERT(pi != nullptr); GB_ASSERT(pi->decl != nullptr); Entity *e = pi->decl->entity; @@ -6337,6 +6340,8 @@ gb_internal void check_safety_all_procedures_for_unchecked(Checker *c) { consume_proc_info(c, pi, &untyped); } } + + array_add(&c->info.all_procedures, pi); } } @@ -7412,7 +7417,6 @@ gb_internal void check_parsed_files(Checker *c) { TIME_SECTION("add untyped expression values"); - // Add untyped expression values for (UntypedExprInfo u = {}; mpsc_dequeue(&c->global_untyped_queue, &u); /**/) { GB_ASSERT(u.expr != nullptr && u.info != nullptr); if (is_type_typed(u.info->type)) { diff --git a/src/checker.hpp b/src/checker.hpp index 8b4d61ee2..5a40b10a0 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -309,11 +309,12 @@ struct EntityGraphNode; typedef PtrSet EntityGraphNodeSet; struct EntityGraphNode { - Entity * entity; // Procedure, Variable, Constant + Entity *entity; // Procedure, Variable, Constant + EntityGraphNodeSet pred; EntityGraphNodeSet succ; - isize index; // Index in array/queue - isize dep_count; + isize index; // Index in array/queue + isize dep_count; }; @@ -516,7 +517,7 @@ struct CheckerInfo { BlockingMutex load_file_mutex; StringMap load_file_cache; - BlockingMutex all_procedures_mutex; + MPSCQueue all_procedures_queue; Array all_procedures; BlockingMutex instrumentation_mutex; @@ -629,7 +630,7 @@ gb_internal void scope_lookup_parent (Scope *s, String const &name, Scope **s gb_internal Entity *scope_insert (Scope *s, Entity *entity); -gb_internal void add_type_and_value (CheckerContext *c, Ast *expression, AddressingMode mode, Type *type, ExactValue const &value); +gb_internal void add_type_and_value (CheckerContext *c, Ast *expression, AddressingMode mode, Type *type, ExactValue const &value, bool use_mutex=true); gb_internal ExprInfo *check_get_expr_info (CheckerContext *c, Ast *expr); gb_internal void add_untyped (CheckerContext *c, Ast *expression, AddressingMode mode, Type *basic_type, ExactValue const &value); gb_internal void add_entity_use (CheckerContext *c, Ast *identifier, Entity *entity); @@ -650,8 +651,8 @@ gb_internal void check_collect_entities(CheckerContext *c, Slice const &n gb_internal void check_collect_entities_from_when_stmt(CheckerContext *c, AstWhenStmt *ws); gb_internal void check_delayed_file_import_entity(CheckerContext *c, Ast *decl); -gb_internal CheckerTypePath *new_checker_type_path(); -gb_internal void destroy_checker_type_path(CheckerTypePath *tp); +gb_internal CheckerTypePath *new_checker_type_path(gbAllocator allocator); +gb_internal void destroy_checker_type_path(CheckerTypePath *tp, gbAllocator allocator); gb_internal void check_type_path_push(CheckerContext *c, Entity *e); gb_internal Entity *check_type_path_pop (CheckerContext *c); diff --git a/src/entity.cpp b/src/entity.cpp index 5ca3fa916..e07e882f3 100644 --- a/src/entity.cpp +++ b/src/entity.cpp @@ -348,7 +348,7 @@ gb_internal Entity *alloc_entity(EntityKind kind, Scope *scope, Token token, Typ entity->type = type; entity->id = 1 + global_entity_id.fetch_add(1); if (token.pos.file_id) { - entity->file = thread_safe_get_ast_file_from_id(token.pos.file_id); + entity->file = thread_unsafe_get_ast_file_from_id(token.pos.file_id); } return entity; } diff --git a/src/error.cpp b/src/error.cpp index 10bf1caf5..53bc01654 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -153,6 +153,17 @@ gb_internal AstFile *thread_safe_get_ast_file_from_id(i32 index) { } +// use AFTER PARSER +gb_internal AstFile *thread_unsafe_get_ast_file_from_id(i32 index) { + GB_ASSERT(index >= 0); + AstFile *file = nullptr; + if (index < global_files.count) { + file = global_files[index]; + } + return file; +} + + // NOTE: defined in build_settings.cpp gb_internal bool global_warnings_as_errors(void); diff --git a/src/threading.cpp b/src/threading.cpp index f1d9264e3..a35176ce6 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -448,9 +448,9 @@ gb_internal void semaphore_wait(Semaphore *s) { } #endif -static const int RWLOCK_WRITER = 1; -static const int RWLOCK_UPGRADED = 2; -static const int RWLOCK_READER = 4; +static const int RWLOCK_WRITER = 1<<0; +static const int RWLOCK_UPGRADED = 1<<1; +static const int RWLOCK_READER = 1<<2; struct RWSpinLock { Futex bits; }; @@ -467,7 +467,7 @@ bool rwlock_try_acquire_upgrade(RWSpinLock *l) { void rwlock_acquire_upgrade(RWSpinLock *l) { while (!rwlock_try_acquire_upgrade(l)) { - futex_wait(&l->bits, RWLOCK_UPGRADED); + futex_wait(&l->bits, RWLOCK_UPGRADED | RWLOCK_WRITER); } } void rwlock_release_upgrade(RWSpinLock *l) { @@ -481,7 +481,7 @@ bool rwlock_try_release_upgrade_and_acquire_write(RWSpinLock *l) { void rwlock_release_upgrade_and_acquire_write(RWSpinLock *l) { while (!rwlock_try_release_upgrade_and_acquire_write(l)) { - futex_wait(&l->bits, RWLOCK_WRITER); + futex_wait(&l->bits, RWLOCK_UPGRADED); } } -- cgit v1.2.3