Replace non-recursive mutexes with `BlockingMutex`; Minor improves to initialization improves

src/check_decl.cpp

@@ -821,7 +821,7 @@ void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) {
 
 		init_entity_foreign_library(ctx, e);
 
-		gb_mutex_lock(&ctx->info->foreign_mutex);
+		mutex_lock(&ctx->info->foreign_mutex);
 
 		auto *fp = &ctx->info->foreigns;
 		StringHashKey key = string_hash_string(name);
@@ -850,14 +850,14 @@ void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) {
 			string_map_set(fp, key, e);
 		}
 
-		gb_mutex_unlock(&ctx->info->foreign_mutex);
+		mutex_unlock(&ctx->info->foreign_mutex);
 	} else {
 		String name = e->token.string;
 		if (e->Procedure.link_name.len > 0) {
 			name = e->Procedure.link_name;
 		}
 		if (e->Procedure.link_name.len > 0 || is_export) {
-			gb_mutex_lock(&ctx->info->foreign_mutex);
+			mutex_lock(&ctx->info->foreign_mutex);
 
 			auto *fp = &ctx->info->foreigns;
 			StringHashKey key = string_hash_string(name);
@@ -876,7 +876,7 @@ void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) {
 				string_map_set(fp, key, e);
 			}
 
-			gb_mutex_unlock(&ctx->info->foreign_mutex);
+			mutex_unlock(&ctx->info->foreign_mutex);
 		}
 	}
 }
@@ -1326,7 +1326,7 @@ void check_proc_body(CheckerContext *ctx_, Token token, DeclInfo *decl, Type *ty
 		if (ps->flags & (ScopeFlag_File & ScopeFlag_Pkg & ScopeFlag_Global)) {
 			return;
 		} else {
-			gb_mutex_lock(&ctx->info->deps_mutex);
+			mutex_lock(&ctx->info->deps_mutex);
 
 			// NOTE(bill): Add the dependencies from the procedure literal (lambda)
 			// But only at the procedure level
@@ -1339,7 +1339,7 @@ void check_proc_body(CheckerContext *ctx_, Token token, DeclInfo *decl, Type *ty
 				ptr_set_add(&decl->parent->type_info_deps, t);
 			}
 
-			gb_mutex_unlock(&ctx->info->deps_mutex);
+			mutex_unlock(&ctx->info->deps_mutex);
 		}
 	}
 }

src/checker.cpp

@@ -234,9 +234,9 @@ Scope *create_scope(CheckerInfo *info, Scope *parent, isize init_elements_capaci
 
 	if (parent != nullptr && parent != builtin_pkg->scope) {
 		// TODO(bill): make this an atomic operation
-		if (info) gb_mutex_lock(&info->scope_mutex);
+		if (info) mutex_lock(&info->scope_mutex);
 		DLIST_APPEND(parent->first_child, parent->last_child, s);
-		if (info) gb_mutex_unlock(&info->scope_mutex);
+		if (info) mutex_unlock(&info->scope_mutex);
 	}
 
 	if (parent != nullptr && parent->flags & ScopeFlag_ContextDefined) {
@@ -832,7 +832,12 @@ void init_universal(void) {
 
 
 void init_checker_info(CheckerInfo *i) {
+#define TIME_SECTION(str) do { debugf("[Subsection] %s\n", str); if (build_context.show_more_timings) timings_start_section(&global_timings, str_lit(str)); } while (0)
+
 	gbAllocator a = heap_allocator();
+
+	TIME_SECTION("checker info: general");
+
 	array_init(&i->definitions,   a);
 	array_init(&i->entities,      a);
 	map_init(&i->global_untyped, a);
@@ -853,18 +858,25 @@ void init_checker_info(CheckerInfo *i) {
 		array_init(&i->identifier_uses, a);
 	}
 
+
+	TIME_SECTION("checker info: mpmc queues");
+
 	mpmc_init(&i->entity_queue, a, 1<<20);
 	mpmc_init(&i->definition_queue, a, 1<<20);
 	mpmc_init(&i->required_global_variable_queue, a, 1<<10);
 
+	TIME_SECTION("checker info: mutexes");
+
 	gb_mutex_init(&i->gen_procs_mutex);
 	gb_mutex_init(&i->gen_types_mutex);
-	gb_mutex_init(&i->type_info_mutex);
-	gb_mutex_init(&i->deps_mutex);
-	gb_mutex_init(&i->identifier_uses_mutex);
-	gb_mutex_init(&i->foreign_mutex);
-	gb_mutex_init(&i->scope_mutex);
 
+	mutex_init(&i->type_info_mutex);
+	mutex_init(&i->deps_mutex);
+	mutex_init(&i->identifier_uses_mutex);
+	mutex_init(&i->foreign_mutex);
+	mutex_init(&i->scope_mutex);
+
+#undef TIME_SECTION
 }
 
 void destroy_checker_info(CheckerInfo *i) {
@@ -888,11 +900,11 @@ void destroy_checker_info(CheckerInfo *i) {
 
 	gb_mutex_destroy(&i->gen_procs_mutex);
 	gb_mutex_destroy(&i->gen_types_mutex);
-	gb_mutex_destroy(&i->type_info_mutex);
-	gb_mutex_destroy(&i->deps_mutex);
-	gb_mutex_destroy(&i->identifier_uses_mutex);
-	gb_mutex_destroy(&i->foreign_mutex);
-	gb_mutex_destroy(&i->scope_mutex);
+	mutex_destroy(&i->type_info_mutex);
+	mutex_destroy(&i->deps_mutex);
+	mutex_destroy(&i->identifier_uses_mutex);
+	mutex_destroy(&i->foreign_mutex);
+	mutex_destroy(&i->scope_mutex);
 }
 
 CheckerContext make_checker_context(Checker *c) {
@@ -937,32 +949,27 @@ void reset_checker_context(CheckerContext *ctx, AstFile *file) {
 
 
 
-bool init_checker(Checker *c, Parser *parser) {
-	c->parser = parser;
-
-	if (global_error_collector.count > 0) {
-		return false;
-	}
+void init_checker(Checker *c) {
+#define TIME_SECTION(str) do { debugf("[Subsection] %s\n", str); if (build_context.show_more_timings) timings_start_section(&global_timings, str_lit(str)); } while (0)
 	gbAllocator a = heap_allocator();
 
+	TIME_SECTION("init checker info");
 	init_checker_info(&c->info);
+
 	c->info.checker = c;
 
+	TIME_SECTION("init proc queues");
 	mpmc_init(&c->procs_with_deferred_to_check, a, 1<<10);
 
-	// NOTE(bill): Is this big enough or too small?
-	isize item_size = gb_max3(gb_size_of(Entity), gb_size_of(Type), gb_size_of(Scope));
-	isize total_token_count = c->parser->total_token_count;
-	isize arena_size = 2 * item_size * total_token_count;
-
-	c->builtin_ctx = make_checker_context(c);
-
 	// NOTE(bill): 1 Mi elements should be enough on average
 	mpmc_init(&c->procs_to_check_queue, heap_allocator(), 1<<20);
 	gb_semaphore_init(&c->procs_to_check_semaphore);
 
 	mpmc_init(&c->global_untyped_queue, a, 1<<20);
-	return true;
+
+	c->builtin_ctx = make_checker_context(c);
+
+#undef TIME_SECTION
 }
 
 void destroy_checker(Checker *c) {
@@ -1075,7 +1082,7 @@ isize type_info_index(CheckerInfo *info, Type *type, bool error_on_failure) {
 		type = t_bool;
 	}
 
-	gb_mutex_lock(&info->type_info_mutex);
+	mutex_lock(&info->type_info_mutex);
 
 	isize entry_index = -1;
 	HashKey key = hash_type(type);
@@ -1098,7 +1105,7 @@ isize type_info_index(CheckerInfo *info, Type *type, bool error_on_failure) {
 		}
 	}
 
-	gb_mutex_unlock(&info->type_info_mutex);
+	mutex_unlock(&info->type_info_mutex);
 
 	if (error_on_failure && entry_index < 0) {
 		compiler_error("Type_Info for '%s' could not be found", type_to_string(type));
@@ -1251,9 +1258,9 @@ void add_entity_use(CheckerContext *c, Ast *identifier, Entity *entity) {
 		identifier->Ident.entity = entity;
 
 		if (c->info->allow_identifier_uses) {
-			gb_mutex_lock(&c->info->identifier_uses_mutex);
+			mutex_lock(&c->info->identifier_uses_mutex);
 			array_add(&c->info->identifier_uses, identifier);
-			gb_mutex_unlock(&c->info->identifier_uses_mutex);
+			mutex_unlock(&c->info->identifier_uses_mutex);
 		}
 
 		String dmsg = entity->deprecated_message;
@@ -1308,6 +1315,14 @@ void add_implicit_entity(CheckerContext *c, Ast *clause, Entity *e) {
 	clause->CaseClause.implicit_entity = e;
 }
 
+void add_type_info_type(CheckerContext *c, Type *t) {
+	void add_type_info_type_internal(CheckerContext *c, Type *t);
+
+	mutex_lock(&c->info->type_info_mutex);
+	add_type_info_type_internal(c, t);
+	mutex_unlock(&c->info->type_info_mutex);
+}
+
 void add_type_info_type_internal(CheckerContext *c, Type *t) {
 	if (t == nullptr) {
 		return;
@@ -1320,9 +1335,6 @@ void add_type_info_type_internal(CheckerContext *c, Type *t) {
 		return;
 	}
 
-	gb_mutex_lock(&c->info->type_info_mutex);
-	defer (gb_mutex_unlock(&c->info->type_info_mutex));
-
 	add_type_info_dependency(c->decl, t);
 
 	auto found = map_get(&c->info->type_info_map, hash_type(t));
@@ -1518,11 +1530,6 @@ void add_type_info_type_internal(CheckerContext *c, Type *t) {
 	}
 }
 
-void add_type_info_type(CheckerContext *c, Type *t) {
-	gb_mutex_lock(&c->info->type_info_mutex);
-	add_type_info_type_internal(c, t);
-	gb_mutex_unlock(&c->info->type_info_mutex);
-}
 
 
 gb_global bool global_procedure_body_in_worker_queue = false;
@@ -3361,6 +3368,8 @@ void check_single_global_entity(Checker *c, Entity *e, DeclInfo *d) {
 }
 
 void check_all_global_entities(Checker *c) {
+	// NOTE(bill): This must be single threaded
+	// Don't bother trying
 	for_array(i, c->info.entities) {
 		Entity *e = c->info.entities[i];
 		DeclInfo *d = e->decl_info;
@@ -3784,9 +3793,9 @@ void check_add_foreign_import_decl(CheckerContext *ctx, Ast *decl) {
 	AttributeContext ac = {};
 	check_decl_attributes(ctx, fl->attributes, foreign_import_decl_attribute, &ac);
 	if (ac.require_declaration) {
-		gb_mutex_lock(&ctx->info->foreign_mutex);
+		mutex_lock(&ctx->info->foreign_mutex);
 		array_add(&ctx->info->required_foreign_imports_through_force, e);
-		gb_mutex_unlock(&ctx->info->foreign_mutex);
+		mutex_unlock(&ctx->info->foreign_mutex);
 		add_entity_use(ctx, nullptr, e);
 	}
 }
@@ -3952,6 +3961,49 @@ bool collect_file_decls(CheckerContext *ctx, Slice<Ast *> const &decls) {
 	return false;
 }
 
+void check_create_file_scopes(Checker *c) {
+	for_array(i, c->parser->packages) {
+		AstPackage *pkg = c->parser->packages[i];
+
+		for_array(j, pkg->files) {
+			AstFile *f = pkg->files[j];
+			string_map_set(&c->info.files, f->fullpath, f);
+
+			create_scope_from_file(nullptr, f);
+		}
+
+		pkg->used = true;
+	}
+}
+
+void check_collect_entities_all(Checker *c) {
+	for_array(i, c->parser->packages) {
+		AstPackage *pkg = c->parser->packages[i];
+
+		for_array(j, pkg->files) {
+			AstFile *f = pkg->files[j];
+			string_map_set(&c->info.files, f->fullpath, f);
+
+			create_scope_from_file(nullptr, f);
+		}
+
+		pkg->used = true;
+	}
+
+	CheckerContext collect_entity_ctx = make_checker_context(c);
+	defer (destroy_checker_context(&collect_entity_ctx));
+
+	CheckerContext *ctx = &collect_entity_ctx;
+
+	for_array(i, c->info.files.entries) {
+		AstFile *f = c->info.files.entries[i].value;
+		reset_checker_context(ctx, f);
+		check_collect_entities(ctx, f->decls);
+		GB_ASSERT(ctx->collect_delayed_decls == false);
+	}
+
+}
+
 void check_import_entities(Checker *c) {
 	Array<ImportGraphNode *> dep_graph = generate_import_dependency_graph(c);
 	defer ({
@@ -4387,7 +4439,7 @@ void check_unchecked_bodies(Checker *c) {
 	}
 }
 
-void check_test_names(Checker *c) {
+void check_test_procedures(Checker *c) {
 	if (build_context.test_names.entries.count == 0) {
 		return;
 	}
@@ -4784,26 +4836,12 @@ void check_parsed_files(Checker *c) {
 		}
 	}
 
+	TIME_SECTION("create file scopes");
+	check_create_file_scopes(c);
+
 	TIME_SECTION("collect entities");
 	// Collect Entities
-	CheckerContext collect_entity_ctx = make_checker_context(c);
-	defer (destroy_checker_context(&collect_entity_ctx));
-	for_array(i, c->parser->packages) {
-		AstPackage *pkg = c->parser->packages[i];
-
-		CheckerContext *ctx = &collect_entity_ctx;
-
-		for_array(j, pkg->files) {
-			AstFile *f = pkg->files[j];
-			string_map_set(&c->info.files, f->fullpath, f);
-
-			create_scope_from_file(nullptr, f);
-			reset_checker_context(ctx, f);
-			check_collect_entities(ctx, f->decls);
-		}
-
-		pkg->used = true;
-	}
+	check_collect_entities_all(c);
 
 	TIME_SECTION("import entities");
 	check_import_entities(c);
@@ -4839,8 +4877,8 @@ void check_parsed_files(Checker *c) {
 	TIME_SECTION("generate minimum dependency set");
 	generate_minimum_dependency_set(c, c->info.entry_point);
 
-	TIME_SECTION("check test names");
-	check_test_names(c);
+	TIME_SECTION("check test procedures");
+	check_test_procedures(c);
 
 	TIME_SECTION("calculate global init order");
 	// Calculate initialization order of global variables

src/checker.hpp

@@ -187,12 +187,13 @@ enum { DEFAULT_SCOPE_CAPACITY = 29 };
 struct Scope {
 	Ast *         node;
 	Scope *       parent;
+	// TODO(bill): make this is an ATOMIC singular linked list
 	Scope *       prev;
 	Scope *       next;
 	Scope *       first_child;
 	Scope *       last_child;
-	StringMap<Entity *> elements;
 
+	StringMap<Entity *> elements;
 	Array<Ast *>    delayed_directives;
 	Array<Ast *>    delayed_imports;
 	PtrSet<Scope *> imported;
@@ -298,10 +299,10 @@ struct CheckerInfo {
 
 	gbMutex gen_procs_mutex; // Possibly recursive
 	gbMutex gen_types_mutex; // Possibly recursive
-	gbMutex type_info_mutex; // NOT recursive
-	gbMutex deps_mutex;      // NOT recursive & Only used in `check_proc_body`
-	gbMutex foreign_mutex;   // NOT recursive
-	gbMutex scope_mutex;     // NOT recursive & Only used in `create_scope`
+	BlockingMutex type_info_mutex; // NOT recursive
+	BlockingMutex deps_mutex;      // NOT recursive & Only used in `check_proc_body`
+	BlockingMutex foreign_mutex;   // NOT recursive
+	BlockingMutex scope_mutex;     // NOT recursive & Only used in `create_scope`
 
 	Map<Array<Entity *> > gen_procs;       // Key: Ast * | Identifier -> Entity
 	Map<Array<Entity *> > gen_types;       // Key: Type *
@@ -313,9 +314,9 @@ struct CheckerInfo {
 	Array<Entity *>     required_foreign_imports_through_force;
 
 	// only used by 'odin query'
-	bool         allow_identifier_uses;
-	gbMutex      identifier_uses_mutex;
-	Array<Ast *> identifier_uses;
+	bool          allow_identifier_uses;
+	BlockingMutex identifier_uses_mutex;
+	Array<Ast *>  identifier_uses;
 
 	// NOTE(bill): These are actually MPSC queues
 	// TODO(bill): Convert them to be MPSC queues
@@ -379,6 +380,8 @@ struct Checker {
 
 	ProcBodyQueue procs_to_check_queue;
 	gbSemaphore procs_to_check_semaphore;
+
+	// TODO(bill): Technically MPSC queue
 	MPMCQueue<UntypedExprInfo> global_untyped_queue;
 };
 

src/common.cpp

@@ -1299,3 +1299,41 @@ Slice<DistanceAndTarget> did_you_mean_results(DidYouMeanAnswers *d) {
 	}
 	return slice_array(d->distances, 0, count);
 }
+
+
+
+#if defined(GB_SYSTEM_WINDOWS)
+	struct BlockingMutex {
+		SRWLOCK srwlock;
+	};
+	void mutex_init(BlockingMutex *m) {
+	}
+	void mutex_destroy(BlockingMutex *m) {
+	}
+	void mutex_lock(BlockingMutex *m) {
+		AcquireSRWLockExclusive(&m->srwlock);
+	}
+	bool mutex_try_lock(BlockingMutex *m) {
+		return !!TryAcquireSRWLockExclusive(&m->srwlock);
+	}
+	void mutex_unlock(BlockingMutex *m) {
+		ReleaseSRWLockExclusive(&m->srwlock);
+	}
+#else
+	typedef gbMutex BlockingMutex;
+	void mutex_init(BlockingMutex *m) {
+		gb_mutex_init(m);
+	}
+	void mutex_destroy(BlockingMutex *m) {
+		gb_mutex_destroy(m);
+	}
+	void mutex_lock(BlockingMutex *m) {
+		gb_mutex_lock(m);
+	}
+	bool mutex_try_lock(BlockingMutex *m) {
+		return !!gb_mutex_try_lock(m);
+	}
+	void mutex_unlock(BlockingMutex *m) {
+		gb_mutex_unlock(m);
+	}
+#endif

src/main.cpp

@@ -1976,6 +1976,8 @@ int main(int arg_count, char const **arg_ptr) {
 	timings_init(&global_timings, str_lit("Total Time"), 2048);
 	defer (timings_destroy(&global_timings));
 
+	TIME_SECTION("initialization");
+
 	arena_init(&permanent_arena, heap_allocator());
 	temp_allocator_init(&temporary_allocator_data, 16*1024*1024);
 	arena_init(&global_ast_arena, heap_allocator());
@@ -2138,9 +2140,11 @@ int main(int arg_count, char const **arg_ptr) {
 	init_universal();
 	// TODO(bill): prevent compiling without a linker
 
+	Parser parser = {0};
+	Checker checker = {0};
+
 	TIME_SECTION("parse files");
 
-	Parser parser = {0};
 	if (!init_parser(&parser)) {
 		return 1;
 	}
@@ -2158,16 +2162,11 @@ int main(int arg_count, char const **arg_ptr) {
 
 	TIME_SECTION("type check");
 
-	Checker checker = {0};
+	checker.parser = &parser;
+	init_checker(&checker);
+	defer (destroy_checker(&checker));
 
-	bool checked_inited = init_checker(&checker, &parser);
-	defer (if (checked_inited) {
-		destroy_checker(&checker);
-	});
-
-	if (checked_inited) {
-		check_parsed_files(&checker);
-	}
+	check_parsed_files(&checker);
 	if (any_errors()) {
 		return 1;
 	}
@@ -2202,10 +2201,6 @@ int main(int arg_count, char const **arg_ptr) {
 		return 0;
 	}
 
-	if (!checked_inited) {
-		return 1;
-	}
-
 	TIME_SECTION("LLVM API Code Gen");
 	lbGenerator gen = {};
 	if (!lb_init_generator(&gen, &checker)) {

src/queue.cpp

@@ -1,7 +1,13 @@
 template <typename T>
 struct MPMCQueueNode {
-	T data;
 	std::atomic<isize> idx;
+	T                  data;
+};
+
+template <typename T>
+struct MPMCQueueNodeNonAtomic {
+	isize idx;
+	T     data;
 };
 
 typedef char CacheLinePad[64];
@@ -27,14 +33,25 @@ struct MPMCQueue {
 
 template <typename T>
 void mpmc_init(MPMCQueue<T> *q, gbAllocator a, isize size) {
+	size = gb_max(size, 8);
 	size = next_pow2_isize(size);
 	GB_ASSERT(gb_is_power_of_two(size));
 
 	gb_mutex_init(&q->mutex);
 	q->mask = size-1;
 	array_init(&q->buffer, a, size);
-	for (isize i = 0; i < size; i++) {
-		q->buffer[i].idx.store(i, std::memory_order_relaxed);
+
+	// NOTE(bill): pretend it's not atomic for performance
+	auto *raw_data = cast(MPMCQueueNodeNonAtomic<T> *)q->buffer.data;
+	for (isize i = 0; i < size; i += 8) {
+		raw_data[i+0].idx = i+0;
+		raw_data[i+1].idx = i+1;
+		raw_data[i+2].idx = i+2;
+		raw_data[i+3].idx = i+3;
+		raw_data[i+4].idx = i+4;
+		raw_data[i+5].idx = i+5;
+		raw_data[i+6].idx = i+6;
+		raw_data[i+7].idx = i+7;
 	}
 }
 
@@ -71,8 +88,10 @@ isize mpmc_enqueue(MPMCQueue<T> *q, T const &data) {
 				gb_mutex_unlock(&q->mutex);
 				return -1;
 			}
+			// NOTE(bill): pretend it's not atomic for performance
+			auto *raw_data = cast(MPMCQueueNodeNonAtomic<T> *)q->buffer.data;
 			for (isize i = old_size; i < new_size; i++) {
-				q->buffer.data[i].idx.store(i, std::memory_order_relaxed);
+				raw_data[i].idx = i;
 			}
 			q->mask = new_size-1;
 			gb_mutex_unlock(&q->mutex);