From 5c18e6f4da41b8f913e47c58f82a64b54c222b15 Mon Sep 17 00:00:00 2001 From: Musa Mahmood Date: Sat, 6 Dec 2025 22:16:46 -0500 Subject: [PATCH] Cleanup TODOs, refactoring, major bug fixes --- lib/Base/Allocator.h | 15 +++---- lib/Base/Arena.cpp | 4 +- lib/Base/Arena_Array.h | 2 +- lib/Base/Array.h | 11 ++++-- lib/Base/Base.h | 1 - lib/Base/ErrorType.cpp | 40 +++++++++++++------ lib/Base/Hash_Functions.h | 5 ++- lib/Base/Hash_Table.h | 31 +-------------- lib/Base/Logger.h | 2 +- lib/Base/Serializer.h | 2 +- lib/Base/String.cpp | 7 ++-- lib/Base/String.h | 5 +-- lib/Graphics.cpp | 2 +- lib/OS/OS_Win32.cpp | 26 +++++++------ lib/OS/OS_Win32_NTFS.cpp | 6 +-- src/Base_Entry_Point.cpp | 2 +- src/Ex1.cpp | 78 +++++++++++++++++++++++-------------- src/ImGui_Supplementary.cpp | 8 ++-- 18 files changed, 128 insertions(+), 119 deletions(-) diff --git a/lib/Base/Allocator.h b/lib/Base/Allocator.h index 5836df8..3873826 100644 --- a/lib/Base/Allocator.h +++ b/lib/Base/Allocator.h @@ -43,8 +43,6 @@ void* internal_alloc (s64 size); void internal_free (void* memory); void* internal_realloc (void* memory, s64 size, s64 old_size); -template force_inline void Initialize (T* memory) { (*memory) = T(); } - template T* New (Allocator allocator, bool initialize=true) { auto memory = (T*)allocator.proc(Allocator_Mode::ALLOCATE, sizeof(T), 0, nullptr, allocator.data); @@ -105,20 +103,19 @@ force_inline void Delete (Allocator allocator, void* memory) { // current allocator on the context. // For Resizes and Deletes, use internal_realloc and internal_free. -template void reset_struct (T* src) { - (*src) = T(); -} +// #NOTE: Initialize and reset_struct are exactly the same! +template force_inline void Initialize (T* memory) { (*memory) = T(); } +template force_inline void reset_struct (T* src) { (*src) = T(); } -template void zero_struct(T* src) { +template void zero_struct (T* src) { memset(src, 0, sizeof(T)); } -template void poison_struct(T* src) { +template void poison_struct (T* src) { memset(src, 0xCD, sizeof(T)); } -template T* copy_struct(T* src) { +template T* copy_struct (T* src) { T* dst = New(false); memcpy(dst, src, sizeof(T)); } - diff --git a/lib/Base/Arena.cpp b/lib/Base/Arena.cpp index 302cd93..b5d567e 100644 --- a/lib/Base/Arena.cpp +++ b/lib/Base/Arena.cpp @@ -209,9 +209,9 @@ struct Auto_Reset { this->starting_point = arena->current_point; } - // #TODO: Implement with ExpandableArena + // #TODO: Implement with ExpandableArena (probably just use the same implementation as Auto_Release?) // Auto_Reset(ExpandableArena* arena_ex) { - // Auto_Reset((Arena*)arena_ex); + // // } ~Auto_Reset() { diff --git a/lib/Base/Arena_Array.h b/lib/Base/Arena_Array.h index 794c766..bdb68aa 100644 --- a/lib/Base/Arena_Array.h +++ b/lib/Base/Arena_Array.h @@ -3,7 +3,7 @@ constexpr s64 ARRAY_ARENA_START_OFFSET = 64; template -struct ArenaArray { // downcasts to an ArrayView. +struct ArenaArray { // #downcasts to an ArrayView. using ValueType = T; s64 count; T* data; diff --git a/lib/Base/Array.h b/lib/Base/Array.h index b9632eb..4693b78 100644 --- a/lib/Base/Array.h +++ b/lib/Base/Array.h @@ -6,7 +6,7 @@ MSVC_RUNTIME_CHECKS_OFF template -struct Array { // downcasts to an ArrayView. +struct Array { // #downcasts to an ArrayView. using ValueType = T; s64 count; T* data; @@ -249,12 +249,17 @@ struct ArrayView { s64 count; T* data; - // #TODO: Add initializers ArrayView from string, ArrayView from ArenaArray - ArrayView(Array array) { // auto-downcast from Array + ArrayView(Array array) { // auto-#downcast from Array count = array.count; data = array.data; } + // Unfortunately we need ArenaArray to be declared ahead. God, I hate C++. + // ArrayView(ArenaArray array) { // auto-#downcast from ArenaArray + // count = array.count; + // data = array.data; + // } + ArrayView() { count = 0; data = nullptr; } ArrayView(s64 new_count, bool initialize=true) { diff --git a/lib/Base/Base.h b/lib/Base/Base.h index db27e3d..9489411 100644 --- a/lib/Base/Base.h +++ b/lib/Base/Base.h @@ -192,7 +192,6 @@ force_inline s64 Next_Power_Of_Two(s64 v) { #define context_allocator() thread_context()->allocator #define context_logger() &thread_context()->logger -// #TODO #constexpr #MATH - make these constexpr // CHECK THAT THESE ARE CORRECT! constexpr f32 TAU = 6.283185f; constexpr f64 TAU_64 = 6.28318530717958648; diff --git a/lib/Base/ErrorType.cpp b/lib/Base/ErrorType.cpp index 5e5746d..d7a4613 100644 --- a/lib/Base/ErrorType.cpp +++ b/lib/Base/ErrorType.cpp @@ -1,6 +1,3 @@ -// #NOTE: To keep things simple, all allocations for Error should be via GPAllocator. -// We really allocate two things: the Error struct and the error string copy. - #define NO_ERROR nullptr enum class ErrorClass: s32 { @@ -19,6 +16,8 @@ struct Error { s32 source_line; string file_path; string function_name; + string thread_name; + f64 timestamp; // Linked list to errors Error* previous_error; // if we're passing errors up the callstack. @@ -70,10 +69,6 @@ Error* new_error (ErrorClass severity, string error_string) { return error; } -// #TODO: // void OS_Log_Error_With_Code // OS-SPECIFIC CALLING GetLastError, etc. -// Then remove all instances of log_error_code_and_string() -// - void Log_Error_2 (string file_path, string function_name, s32 line_number, ErrorClass severity, string fmt, ...) { auto tctx = thread_context(); Assert(tctx != nullptr); @@ -100,10 +95,30 @@ void Log_Error_2 (string file_path, string function_name, s32 line_number, Error // Note: we don't need to assign previous_error or next_error, as that is done by the thread_context when we #push_error error->previous_error = nullptr; error->next_error = nullptr; + error->thread_name = copy_string(tctx->thread_name); + error->timestamp = GetUnixTimestamp(); push_error(tctx, error); } +Error* copy_error (Thread_Context* tctx, Error* old_error) { + push_arena(tctx->error_arena); + + Error* error = new_error(old_error->severity, to_string(old_error)); + + error->thread_id = old_error->thread_id; + error->source_line = old_error->source_line; + error->file_path = copy_string(old_error->file_path); + error->function_name = copy_string(old_error->function_name); + // Note: we don't need to assign previous_error or next_error, as that is done by the thread_context when we #push_error + error->previous_error = nullptr; + error->next_error = nullptr; + error->thread_name = copy_string(old_error->thread_name); + error->timestamp = old_error->timestamp; + + return error; +} + void push_error (Thread_Context* tctx, Error* new_error) { Assert(tctx == thread_context()); // Not a real assert, just wondering if we'll ever call this with a non-local context? Assert(new_error != nullptr); @@ -191,7 +206,8 @@ void push_errors_to_parent_thread (Thread_Context* tctx) { Assert(tctx->parent_thread_context); while (tctx->first_error) { - push_error_no_context(tctx->parent_thread_context, tctx->first_error); + Error* error_copy = copy_error(tctx->parent_thread_context, tctx->first_error); + push_error_no_context(tctx->parent_thread_context, error_copy); clear_error(tctx, tctx->first_error); } } @@ -206,9 +222,11 @@ ArrayView get_all_errors (Thread_Context* tctx) { } - for_each(t, tctx->child_threads) { - // #TODO: also recurse through child threads? - } + // #TODO(Low priority): also recurse through child threads? + // NOTE: I don't think we actually want this, because we merge + // our errors on the main thread when we thread_deinit. + // for_each(t, tctx->child_threads) { + // } return error_array; } diff --git a/lib/Base/Hash_Functions.h b/lib/Base/Hash_Functions.h index eec747a..9d6cbbd 100644 --- a/lib/Base/Hash_Functions.h +++ b/lib/Base/Hash_Functions.h @@ -51,8 +51,9 @@ u32 table_hash_function_knuth (void* key, s64 size) { u32 string_hash_function_fnv1a (void* key, s64 size) { Assert(size == sizeof(string)); string key_as_string = *((string*)key); - // #TODO It should be xor folded to the desired range rather than shifted. - return (u32)(fnv1a_hash_any(key_as_string.data, key_as_string.count)); + u64 hash_u64 = fnv1a_hash_any(key_as_string.data, key_as_string.count); + // It should be xor folded to the desired range rather than shifted: + return (u32)(hash_u64 ^ (hash_u64 >> 32)); } bool u64_keys_match (void* key1, void* key2) { diff --git a/lib/Base/Hash_Table.h b/lib/Base/Hash_Table.h index 4fa372a..7765133 100644 --- a/lib/Base/Hash_Table.h +++ b/lib/Base/Hash_Table.h @@ -1,30 +1,5 @@ // #NOTE: This Hash Table is borrowed from Jai's implementation! (With some tweaks) // I made my own version that's arena-backed, but the mechanisms are the same. - -// s32 drive_count = 0; -// for (s64 i = 0; i < drive_table->allocated; i += 1) { -// Table_Entry* entry = &drive_table->entries[i]; // we should take ptr here if we want to modify? -// if (entry->hash > HASH_TABLE_FIRST_VALID_HASH) { -// // #TODO #MOVE THIS + maybe don't check this every frame! -// // entry->value.is_present = Win32_Drive_Exists(entry->value.label); -// if (entry->value.label.data == nullptr) continue; - -// Text(" > [%d] drive letter: %s (is_present: %d)", drive_count + 1, entry->value.label.data, entry->value.is_present); -// drive_count += 1; -// SameLine(); -// push_allocator(temp()); - -// if (Button(format_cstring("Read NTFS MFT Raw##%s", entry->value.label.data))) { -// push_arena(thread_context()->arena); -// // auto_release(thread_context()->arena); -// auto dfs = NTFS_MFT_read_raw(entry->value.label); -// if (dfs == nullptr) { -// log("[NTFS_MFT_read_raw] operation failed"); -// } -// } -// } -// } - typedef u32 hash_result; constexpr hash_result HASH_TABLE_FIRST_VALID_HASH = 2; @@ -198,8 +173,6 @@ template void table_resize (Table* table, s64 slo return; } - // #TODO: When you resize you need to reinsert all the values, so we - // need a temporary copy of the original data: ArrayView> old_entries = table->entries; s64 n = Next_Power_Of_Two(slots_to_allocate); @@ -290,10 +263,10 @@ template bool table_remove (Table* table, T key, return false; } -// #TODO: We should allow setting an allocator instead of defaulting to temp()? template ArrayView table_find_multiple (Table* table, T key, U* value) { Array results; - results.allocator = temp(); + // #NOTE: We should allow setting an allocator instead of defaulting to temp()? + // results.allocator = temp(); // #Walk_Table u32 mask = (u32)(table->allocated - 1); diff --git a/lib/Base/Logger.h b/lib/Base/Logger.h index e79fc8e..ab584f4 100644 --- a/lib/Base/Logger.h +++ b/lib/Base/Logger.h @@ -1,4 +1,4 @@ -// #TODO #Logger module +// #TODO(Low priority) #Logger module // [ ] Add colored prints (See: Print_Color.jai) // See Logger.jai in our jiim-dev-gui project for how to do fancy colored text. diff --git a/lib/Base/Serializer.h b/lib/Base/Serializer.h index adb3459..3767186 100644 --- a/lib/Base/Serializer.h +++ b/lib/Base/Serializer.h @@ -54,7 +54,7 @@ force_inline void AddString16 (Serializer* serializer, string s) { AddArray_NoSize(serializer, to_view(s)); } -struct Deserializer { // downcasts to ArrayView +struct Deserializer { // #downcasts to ArrayView s64 count; u8* data; s64 cursor; diff --git a/lib/Base/String.cpp b/lib/Base/String.cpp index be37671..639ce81 100644 --- a/lib/Base/String.cpp +++ b/lib/Base/String.cpp @@ -1,6 +1,5 @@ -// #TODO #string module -// [ ] I'm debating if string type should automatically null-terminate. -// I personally do not like it, and think we should temp-copy c-strings as they're needed. +// #NOTE: All string building, printing and copying operations SHOULD null-terminate the +// strings for backwards compatibility reasons. #FIX if something doesn't follow this rule! bool is_valid (string s) { return (s.data != nullptr && s.count > 0); } @@ -309,4 +308,4 @@ string trim_right (string s, string chars, bool replace_with_zeros) { } return string_view(s, 0, count); -} \ No newline at end of file +} diff --git a/lib/Base/String.h b/lib/Base/String.h index c850c7a..4a1f0bc 100644 --- a/lib/Base/String.h +++ b/lib/Base/String.h @@ -1,7 +1,7 @@ #pragma once // #TODO: #strings: - // [x] Always null-terminate strings! - // [ ] How do I accept variadic arguments of any type to my print function? + // [ ] see: #Parsing stuff: + // [?] How do I accept variadic arguments of any type to my print function? // [ ] Need to sort out how formatted strings and string builders are allocated // [ ] Separate functions for temp alloc (tprint??) // [ ] I should also put path manipulation here or in a separate file? @@ -82,7 +82,6 @@ force_inline string string_view (string s, s64 start_index, s64 view_count); bool strings_match (string first_string, string second_string); // #Unicode -// #TODO: Make a raw version that returns the raw pointer? string wide_to_utf8 (u16* source, s32 length=-1); wstring utf8_to_wide (string source); diff --git a/lib/Graphics.cpp b/lib/Graphics.cpp index 1569d1f..490d70f 100644 --- a/lib/Graphics.cpp +++ b/lib/Graphics.cpp @@ -82,6 +82,6 @@ void graphics_set_render_target (Window_Info window_info) { graphics->texture_render_target = nullptr; graphics->current_window = window_info; - // #TODO: graphics_update_window :: + // #TODO: #Graphics graphics_update_window :: } \ No newline at end of file diff --git a/lib/OS/OS_Win32.cpp b/lib/OS/OS_Win32.cpp index 08a3cb0..73566a1 100644 --- a/lib/OS/OS_Win32.cpp +++ b/lib/OS/OS_Win32.cpp @@ -87,7 +87,7 @@ ArrayView os_get_available_drives () { Table_Entry* entry = &drive_table->entries[i]; // we should take ptr here if we want to modify? if (entry->hash > HASH_TABLE_FIRST_VALID_HASH) { - if (entry->value->label.data == nullptr) continue; // Some volumes may not be real and therefore have no label. + if (entry->value->label.data == nullptr || !entry->value->is_present) continue; // Some volumes may not be real and therefore have no label. array_add(drives, entry->value); } @@ -264,13 +264,15 @@ internal bool thread_init (Thread* thread, Thread_Proc proc, string thread_name= s64 this_thread_index = InterlockedIncrement(&next_thread_index); // 2. #NewContext Setup NEW thread local context ExpandableArena* arena_ex = expandable_arena_new(Arena_Reserve::Size_64M, 16); + push_arena(arena_ex); // #NOTE: we don't assign thread_local_context until we hit the #thread_entry_point - thread->context = New(allocator(arena_ex)); + thread->context = New(); thread->context->temp = expandable_arena_new(Arena_Reserve::Size_2M, 16); thread->context->arena = arena_ex; thread->context->allocator = allocator(arena_ex); thread->context->thread_idx = (s32)this_thread_index; + // #NOTE: This will disappear once the thread is de-initted. If we want this string, copy it! thread->context->thread_name = copy_string(thread_name); thread->context->log_builder = new_string_builder(Arena_Reserve::Size_64M); thread->context->error_arena = next_arena(Arena_Reserve::Size_64M); @@ -296,8 +298,8 @@ internal void thread_deinit (Thread* thread, bool zero_thread=false) { if (thread->os_thread.windows_thread) { CloseHandle(thread->os_thread.windows_thread); + thread->os_thread.windows_thread = nullptr; } - thread->os_thread.windows_thread = nullptr; array_reset(*thread->context->log_builder); free_string_builder(thread->context->log_builder); @@ -392,7 +394,7 @@ internal string get_error_string (OS_Error_Code error_code) { return trim_right(result); } -internal void log_error_code_and_string () { // #TODO: replace with call to log_error_with_code +internal void os_log_error () { OS_Error_Code error_code = GetLastError(); log_error(" > GetLastError code: %d, %s", error_code, get_error_string(error_code).data); } @@ -428,7 +430,7 @@ internal File file_open (string file_path, bool for_writing, bool keep_existing_ if (handle == INVALID_HANDLE_VALUE && log_errors) { // OS_Error_Code error_code = GetLastError(); log_error("Could not open file `%s`", file_path); - log_error_code_and_string(); + os_log_error(); } File file; @@ -445,7 +447,7 @@ internal bool file_read (File file, u8* data, s64 bytes_to_read_count, s64* byte // ignore bytes_read_count if null. if (data == nullptr) { log_error("file_read called with null destination pointer."); - log_error_code_and_string(); + os_log_error(); if (bytes_read_count) (*bytes_read_count) = 0; return false; } @@ -624,8 +626,8 @@ monitor_enum_proc (HMONITOR hMonitor, HDC hdc, RECT* rect, LPARAM data) { return true; } -// #TODO: how do I setup a callback if monitors configuration is changed? -// we handle WM_DISPLAYCHANGE or WM_DEVICECHANGE and call EnumDisplayMonitors +// #TODO(Low Priority): how do I setup a callback if monitors configuration is changed? +// we handle WM_DISPLAYCHANGE or WM_DEVICECHANGE and call EnumDisplayMonitors again. internal void os_enumerate_monitors () { if (!global_win32_state.system_info.monitors_enumerated) { // should reset array? @@ -979,7 +981,7 @@ bool Win32_Discover_Drives () { Table* drive_table = get_drive_table(); if (!drive_table->allocated) { drive_table->allocator = GPAllocator(); - // #TODO: #hash_table need a macro for string keys! + // #TODO(Low priority): #hash_table need a macro for initializing with string keys! drive_table->hash_function = string_hash_function_fnv1a; drive_table->compare_function = string_keys_match; s64 slots_to_allocate = 64; @@ -990,7 +992,7 @@ bool Win32_Discover_Drives () { u32 result_length = GetLogicalDriveStringsW(1024, (LPWSTR)lpBuf); if (!result_length) { log_error("GetLogicalDriveStringsW failed!"); - log_error_code_and_string(); + os_log_error(); return false; } @@ -1048,7 +1050,7 @@ bool Win32_Discover_Drives () { log(" - file_system: %s", wide_to_utf8(file_system_name).data); } else { log_error("GetVolumeInformationW failed! (drive label: %s)", drive_label.data); - log_error_code_and_string(); + os_log_error(); drive->is_present = false; } } @@ -1072,7 +1074,7 @@ string Win32_drive_letter (string any_path) { return copy_string({1, any_path.data}); } -// #TODO: #window_creation +// #TODO: #window_creation #window_manipulation // [ ] resize_window // [ ] position_window // [ ] toggle_fullscreen diff --git a/lib/OS/OS_Win32_NTFS.cpp b/lib/OS/OS_Win32_NTFS.cpp index 7cd934a..5f19a3e 100644 --- a/lib/OS/OS_Win32_NTFS.cpp +++ b/lib/OS/OS_Win32_NTFS.cpp @@ -164,7 +164,7 @@ Error* NTFS_MFT_read_raw (OS_Drive* drive) { if (file_handle == INVALID_HANDLE_VALUE) { log_error("CreateFileA failed on target %s", create_file_target.data); - log_error_code_and_string(); + os_log_error(); return nullptr; } @@ -272,7 +272,7 @@ Error* NTFS_MFT_read_raw (OS_Drive* drive) { file.is_directory = fileRecord->isDirectory; // We need to get size from the data attribute - // #TODO: continue from here! + // #TODO: #continue from here! mft->file_count += 1; } } @@ -292,7 +292,7 @@ Error* NTFS_MFT_read_raw (OS_Drive* drive) { CloseHandle(file_handle); - log("Found %lld files (bytes_accessed: %s)", mft->file_count, format_bytes(mft->bytes_accessed).data); + log_none("Found %lld files (bytes_accessed: %s)", mft->file_count, format_bytes(mft->bytes_accessed).data); drive->file_count = mft->file_count; drive->bytes_accessed = mft->bytes_accessed; diff --git a/src/Base_Entry_Point.cpp b/src/Base_Entry_Point.cpp index 02bccc6..c0d2b8e 100644 --- a/src/Base_Entry_Point.cpp +++ b/src/Base_Entry_Point.cpp @@ -44,7 +44,7 @@ internal void Main_Entry_Point (int argc, WCHAR **argv) { // #entry_point graphics_set_render_target(get_main_window()); // 3. Init Graphics (DX11 or OpenGL3) - // #TODO: #Main - `Main_Entry_Point` + // #TODO: #Graphics#Main - `Main_Entry_Point` // 4. [ ] Setup Mouse and Keyboard Inputs // 5. [ ] Launch second thread; thread groups diff --git a/src/Ex1.cpp b/src/Ex1.cpp index 12d3ff6..aa389bf 100644 --- a/src/Ex1.cpp +++ b/src/Ex1.cpp @@ -12,7 +12,9 @@ struct Explorer { }; struct Ex1_NTFS_Enumeration { - bool initialized; + b32 initialized; + b32 threads_started; + ArrayView threads; Array threads_in_flight; }; @@ -22,6 +24,19 @@ global ExplorerUI explorer_ui; global Explorer explorer; global Ex1_NTFS_Enumeration ex1_ntfs; +void ntfs_create_enumeration_threads (s32 thread_count) { + if (!ex1_ntfs.initialized) { Timed_Block_Print("Thread initialization (ntfs)"); + ex1_ntfs.initialized = true; + ex1_ntfs.threads = ArrayView(thread_count); + ex1_ntfs.threads_in_flight.allocator = GPAllocator(); + for_each(t, ex1_ntfs.threads) { + string thread_name = format_string("ntfs_enumeration_thread#%d", t); + bool success = thread_init(&ex1_ntfs.threads[t], ntfs_enumeration_thread_proc, thread_name); + Assert(success); + } + } +} + #define HOTKEY_ID_BRING_TO_FOREGROUND 1 #define VK_SPACE_KEY_CODE 0x20 // #define HOTKEY_ID_HIDE_TITLEBAR @@ -67,13 +82,14 @@ void Ex1_Control_Panel () { using namespace ImGui; Table* drive_table = get_drive_table(); Begin("Control Panel"); + if (Button("Debug break")) { debug_break(); } if (Button("Discover drives") || !table_is_valid(drive_table)) { Win32_Discover_Drives(); } Text("drive_table is valid: %d", table_is_valid(drive_table)); push_allocator(temp()); - ArrayView drives = os_get_available_drives(); + ArrayView drives = os_get_available_drives(); // only includes drives that are ready. for_each(i, drives) { OS_Drive* drive = drives[i]; @@ -90,36 +106,24 @@ void Ex1_Control_Panel () { using namespace ImGui; // } } - if (drives.count > 0) { - if (!ex1_ntfs.initialized) { Timed_Block_Print("Thread initialization (ntfs)"); - push_allocator(GPAllocator()); - ex1_ntfs.initialized = true; - s32 cpu_core_count = os_cpu_physical_core_count(); - ex1_ntfs.threads = ArrayView(cpu_core_count); - ex1_ntfs.threads_in_flight.allocator = GPAllocator(); - for_each(t, ex1_ntfs.threads) { - string thread_name = format_string("ntfs_enumeration_thread#%d", t); - bool success = thread_init(&ex1_ntfs.threads[t], ntfs_enumeration_thread_proc, thread_name); - Assert(success); - } - } - } - - if (drives.count > 0 && ex1_ntfs.initialized && Button("Enumerate all NTFS drives")) { + if (drives.count > 0 && Button("Enumerate all NTFS drives")) { // && ex1_ntfs.initialized // if drive count exceeds the number of threads, we need to group them so each thread // can enumerate multiple drives. // We need to distribute the drives across our available threads: - // #TODO #is_present - drive_count should only include if `drive->is_present`. + push_allocator(GPAllocator()); Array> drive_split; - s32 thread_count = (s32)ex1_ntfs.threads.count; - drive_split.allocator = GPAllocator(); - - if (drives.count > thread_count) { + drive_split.allocator = temp(); // this is only needed for this frame + + if (drives.count > os_cpu_physical_core_count()) { + s32 thread_count = os_cpu_physical_core_count(); + array_resize(drive_split, thread_count); + ntfs_create_enumeration_threads(thread_count); + + s32 threads_to_create = thread_count; s64 drives_per_thread = (drives.count / thread_count); s64 remainder = drives.count % thread_count; s64 current_drive = 0; - push_allocator(GPAllocator()); - array_resize(drive_split, thread_count); + for_each(d, drive_split) { if (d == drive_split.count) { drive_split[d] = ArrayView(remainder); @@ -135,7 +139,9 @@ void Ex1_Control_Panel () { using namespace ImGui; debug_break(); // #TODO: Check that the work has been distributed correctly. } else { // more threads than drives, or same amount + s32 thread_count = (s32)drives.count; array_resize(drive_split, drives.count); + ntfs_create_enumeration_threads(thread_count); for_each(d, drives) { auto drive = drives[d]; @@ -147,6 +153,7 @@ void Ex1_Control_Panel () { using namespace ImGui; s64 active_thread_count = drive_split.count; + ex1_ntfs.threads_started = true; for (s64 t = 0; t < active_thread_count; t += 1) { Thread* thread = &ex1_ntfs.threads[t]; Arena* thread_arena = next_arena(Arena_Reserve::Size_64K); @@ -159,19 +166,30 @@ void Ex1_Control_Panel () { using namespace ImGui; } } + if (ex1_ntfs.threads_started && !ex1_ntfs.threads_in_flight.count) { + // All threads are complete, we're free to clean up remaining memory + push_allocator(GPAllocator()); + array_free(ex1_ntfs.threads); + array_free(ex1_ntfs.threads_in_flight); + + // Instead maybe we should just memset this to zero. + reset_struct(&ex1_ntfs); + } + if (ex1_ntfs.threads_in_flight.count) { Text("Threads in flight: %d", ex1_ntfs.threads_in_flight.count); - // keep track of which indices to remove! + for_each(t, ex1_ntfs.threads_in_flight) { if (thread_is_done(ex1_ntfs.threads_in_flight[t])) { + push_allocator(GPAllocator()); Thread* thread = ex1_ntfs.threads_in_flight[t]; auto task = thread_task(NTFS_Enumeration_Task); + array_free(task->drives); + // internal_free( + // make sure to retreive any data you need to from here! release_arena(task->pool); - // #TODO Before #deiniting thread we SHOULD copy the errors from the - // thread before it's deleted forever. - - thread_deinit(ex1_ntfs.threads_in_flight[t], true); + thread_deinit(ex1_ntfs.threads_in_flight[t], false); array_unordered_remove_by_index(ex1_ntfs.threads_in_flight, t); t -= 1; // check this element index again! } diff --git a/src/ImGui_Supplementary.cpp b/src/ImGui_Supplementary.cpp index 23e429c..b5a9eb8 100644 --- a/src/ImGui_Supplementary.cpp +++ b/src/ImGui_Supplementary.cpp @@ -298,6 +298,7 @@ global f32 imgui_font_sizes[6] = {18, 24, 30, 36, 42, 48}; global ImGui_Font_Info imgui_default_font; bool ImGui_Default_Font_Loaded () { + // #TODO #font_crash Should only check once at the top of the frame! return imgui_default_font.sizes.count > 0; } @@ -306,10 +307,6 @@ f32 ImGui_Default_Font_Current_Size () { } void ImGui_Push_Default_Font () { - // #TODO: Replace these asserts so it just returns if not present! - // And just warn in the UI that a font is missing. - Assert(imgui_default_font.sizes.count > 0); - Assert(imgui_default_font.current_size >= 0 && imgui_default_font.current_size < imgui_default_font.sizes.count); if (ImGui_Default_Font_Loaded()) { ImFont* font_data = imgui_default_font.sizes[imgui_default_font.current_size].data; f32 unscaled_font_size = imgui_default_font.sizes[imgui_default_font.current_size].font_size; @@ -318,7 +315,8 @@ void ImGui_Push_Default_Font () { } void ImGui_Pop_Default_Font () { - // This will break if we load font mid-frame! don't do this. + // #font_crash: This will break if we load font mid-frame! don't do this. + // We should only check this once at the top of the frame! if (ImGui_Default_Font_Loaded()) { ImGui::PopFont(); }