From 41b32f0da4b40295771e2a9a521b89464d98201b Mon Sep 17 00:00:00 2001 From: gingerBill Date: Thu, 22 Dec 2022 12:45:23 +0000 Subject: [PATCH] Clean up mutex usage in the parser --- src/parser.cpp | 62 ++++++++++++++++++++++++-------------------------- src/parser.hpp | 51 +++++++++++++++++++++++------------------ 2 files changed, 59 insertions(+), 54 deletions(-) diff --git a/src/parser.cpp b/src/parser.cpp index 436498c51..cb9713985 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -4858,13 +4858,10 @@ gb_internal bool init_parser(Parser *p) { GB_ASSERT(p != nullptr); string_set_init(&p->imported_files, heap_allocator()); array_init(&p->packages, heap_allocator()); - array_init(&p->package_imports, heap_allocator()); - mutex_init(&p->wait_mutex); - mutex_init(&p->import_mutex); - mutex_init(&p->file_add_mutex); + mutex_init(&p->imported_files_mutex); mutex_init(&p->file_decl_mutex); mutex_init(&p->packages_mutex); - mpmc_init(&p->file_error_queue, heap_allocator(), 1024); + mutex_init(&p->file_error_mutex); return true; } @@ -4879,20 +4876,12 @@ gb_internal void destroy_parser(Parser *p) { array_free(&pkg->files); array_free(&pkg->foreign_files); } -#if 0 - for_array(i, p->package_imports) { - // gb_free(heap_allocator(), p->package_imports[i].text); - } -#endif array_free(&p->packages); - array_free(&p->package_imports); string_set_destroy(&p->imported_files); - mutex_destroy(&p->wait_mutex); - mutex_destroy(&p->import_mutex); - mutex_destroy(&p->file_add_mutex); + mutex_destroy(&p->imported_files_mutex); mutex_destroy(&p->file_decl_mutex); mutex_destroy(&p->packages_mutex); - mpmc_destroy(&p->file_error_queue); + mutex_destroy(&p->file_error_mutex); } @@ -4909,7 +4898,18 @@ gb_internal WORKER_TASK_PROC(parser_worker_proc) { ParserWorkerData *wd = cast(ParserWorkerData *)data; ParseFileError err = process_imported_file(wd->parser, wd->imported_file); if (err != ParseFile_None) { - mpmc_enqueue(&wd->parser->file_error_queue, err); + auto *node = gb_alloc_item(permanent_allocator(), ParseFileErrorNode); + node->err = err; + + mutex_lock(&wd->parser->file_error_mutex); + if (wd->parser->file_error_tail != nullptr) { + wd->parser->file_error_tail->next = node; + } + wd->parser->file_error_tail = node; + if (wd->parser->file_error_head == nullptr) { + wd->parser->file_error_head = node; + } + mutex_unlock(&wd->parser->file_error_mutex); } return cast(isize)err; } @@ -4926,7 +4926,6 @@ gb_internal void parser_add_file_to_process(Parser *p, AstPackage *pkg, FileInfo gb_internal WORKER_TASK_PROC(foreign_file_worker_proc) { ForeignFileWorkerData *wd = cast(ForeignFileWorkerData *)data; - Parser *p = wd->parser; ImportedFile *imp = &wd->imported_file; AstPackage *pkg = imp->pkg; @@ -4946,9 +4945,9 @@ gb_internal WORKER_TASK_PROC(foreign_file_worker_proc) { // TODO(bill): Actually do something with it break; } - mutex_lock(&p->file_add_mutex); + mutex_lock(&pkg->foreign_files_mutex); array_add(&pkg->foreign_files, foreign_file); - mutex_unlock(&p->file_add_mutex); + mutex_unlock(&pkg->foreign_files_mutex); return 0; } @@ -4968,7 +4967,7 @@ gb_internal void parser_add_foreign_file_to_process(Parser *p, AstPackage *pkg, gb_internal AstPackage *try_add_import_path(Parser *p, String const &path, String const &rel_path, TokenPos pos, PackageKind kind = Package_Normal) { String const FILE_EXT = str_lit(".odin"); - MUTEX_GUARD_BLOCK(&p->import_mutex) { + MUTEX_GUARD_BLOCK(&p->imported_files_mutex) { if (string_set_update(&p->imported_files, path)) { return nullptr; } @@ -4979,6 +4978,9 @@ gb_internal AstPackage *try_add_import_path(Parser *p, String const &path, Strin pkg->fullpath = path; array_init(&pkg->files, heap_allocator()); pkg->foreign_files.allocator = heap_allocator(); + mutex_init(&pkg->files_mutex); + mutex_init(&pkg->foreign_files_mutex); + // NOTE(bill): Single file initial package if (kind == Package_Init && string_ends_with(path, FILE_EXT)) { @@ -5716,10 +5718,9 @@ gb_internal ParseFileError process_imported_file(Parser *p, ImportedFile importe } if (parse_file(p, file)) { - mutex_lock(&p->file_add_mutex); - defer (mutex_unlock(&p->file_add_mutex)); - - array_add(&pkg->files, file); + MUTEX_GUARD_BLOCK(&pkg->files_mutex) { + array_add(&pkg->files, file); + } if (pkg->name.len == 0) { pkg->name = file->package_name; @@ -5733,8 +5734,8 @@ gb_internal ParseFileError process_imported_file(Parser *p, ImportedFile importe } } - p->total_line_count += file->tokenizer.line_count; - p->total_token_count += file->tokens.count; + p->total_line_count.fetch_add(file->tokenizer.line_count); + p->total_token_count.fetch_add(file->tokens.count); } return ParseFile_None; @@ -5771,9 +5772,6 @@ gb_internal ParseFileError parse_packages(Parser *p, String init_filename) { { // Add these packages serially and then process them parallel - mutex_lock(&p->wait_mutex); - defer (mutex_unlock(&p->wait_mutex)); - TokenPos init_pos = {}; { String s = get_fullpath_core(heap_allocator(), str_lit("runtime")); @@ -5808,9 +5806,9 @@ gb_internal ParseFileError parse_packages(Parser *p, String init_filename) { global_thread_pool_wait(); - for (ParseFileError err = ParseFile_None; mpmc_dequeue(&p->file_error_queue, &err); /**/) { - if (err != ParseFile_None) { - return err; + for (ParseFileErrorNode *node = p->file_error_head; node != nullptr; node = node->next) { + if (node->err != ParseFile_None) { + return node->err; } } diff --git a/src/parser.hpp b/src/parser.hpp index df882cc0f..4357573c9 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -62,15 +62,6 @@ enum PackageKind { Package_Init, }; -struct ImportedPackage { - PackageKind kind; - String path; - String rel_path; - TokenPos pos; // import - isize index; -}; - - struct ImportedFile { AstPackage *pkg; FileInfo fi; @@ -182,6 +173,9 @@ struct AstPackage { bool is_single_file; isize order; + BlockingMutex files_mutex; + BlockingMutex foreign_files_mutex; + MPMCQueue exported_entity_queue; // NOTE(bill): Created/set in checker @@ -191,20 +185,33 @@ struct AstPackage { }; +struct ParseFileErrorNode { + ParseFileErrorNode *next, *prev; + ParseFileError err; +}; + struct Parser { - String init_fullpath; - StringSet imported_files; // fullpath - Array packages; - Array package_imports; - isize file_to_process_count; - isize total_token_count; - isize total_line_count; - BlockingMutex wait_mutex; - BlockingMutex import_mutex; - BlockingMutex file_add_mutex; - BlockingMutex file_decl_mutex; - BlockingMutex packages_mutex; - MPMCQueue file_error_queue; + String init_fullpath; + + StringSet imported_files; // fullpath + BlockingMutex imported_files_mutex; + + Array packages; + BlockingMutex packages_mutex; + + std::atomic file_to_process_count; + std::atomic total_token_count; + std::atomic total_line_count; + + // TODO(bill): What should this mutex be per? + // * Parser + // * Package + // * File + BlockingMutex file_decl_mutex; + + BlockingMutex file_error_mutex; + ParseFileErrorNode * file_error_head; + ParseFileErrorNode * file_error_tail; }; struct ParserWorkerData {