Closing technical debt

- Manage resources deallocations in AssetManager
- Reformatting in AssetManager
- Fix resource address synchronization in AssetManager
- Windows native file system uses IOCP for performance

engine/modules/AssetManager.cppm

@@ -28,11 +28,11 @@ export
         ee_assets::AssetType type = ee_assets::BINARY;
 
         std::atomic<bool> isReady;
-        AssetAddress *resourceAddress;
+        std::atomic<AssetAddress*> resourceAddress;
 
         explicit AssetInfo(void *resource) {
             this->resourceAddress = Allocate<AssetAddress>();
-            this->resourceAddress->resource.store(resource);
+            this->resourceAddress.load(std::memory_order_relaxed)->resource.store(resource);
         }
 
         AssetInfo(const AssetInfo&) = delete;
@@ -40,7 +40,7 @@ export
 
         AssetInfo(AssetInfo&& other) noexcept :
             type(other.type),
-            resourceAddress(other.resourceAddress) {
+            resourceAddress(other.resourceAddress.load(std::memory_order_relaxed)) {
             isReady.store(other.isReady.load(std::memory_order_relaxed), std::memory_order_relaxed);
 
             other.resourceAddress = nullptr;
@@ -50,7 +50,7 @@ export
         AssetInfo& operator=(AssetInfo&& other) noexcept {
             if (this != &other) {
                 type = other.type;
-                resourceAddress = other.resourceAddress;
+                resourceAddress = other.resourceAddress.load(std::memory_order_relaxed);
                 isReady.store(other.isReady.load(std::memory_order_relaxed), std::memory_order_relaxed);
 
                 other.resourceAddress = nullptr;
@@ -105,7 +105,7 @@ export
             }
         }
 
-        void LoadAsync(ee_assets::AssetType type, const std::filesystem::path& path) {
+        void LoadAsync(const ee_assets::AssetType type, const std::filesystem::path& path) {
             AssetStreamCommand command;
             command.type = AssetStreamCommand::LOAD;
             command.targetPath = path;
@@ -150,21 +150,22 @@ export
             return Get<T>(Find(id));
         }
 
-        void Redirect(const AssetHandle& handle, void *resource) const {
+        // ReSharper disable once CppMemberFunctionMayBeConst
+        void Redirect(const AssetHandle& handle, void *resource) {
             const AssetInfo& asset = activeAssets[handle.slot];
-            if (asset.resourceAddress == nullptr) {
+            AssetAddress *address = asset.resourceAddress.load(std::memory_order_relaxed);
+            if (address == nullptr) {
                 LOG(WARNING, "Attempt to redirect empty handle");
                 return;
             }
 
-            // TODO: LEAK! Free old resource after redirecting handler to other resource
+            if (
-            if (asset.resourceAddress->resource.load() != nullptr) {
+                void *oldResource = address->resource.exchange(
-                void *oldResource = asset.resourceAddress->resource.exchange(nullptr, std::memory_order_acq_rel);
+                    resource,
-                //if (oldResource)
+                    std::memory_order_acq_rel
-                //Deallocate(oldResource);
+                )
-            }
+            )
-
+                Deallocate(oldResource);
-            asset.resourceAddress->resource.store(resource);
         }
 
         // Returned resource reference should not be stored
@@ -184,7 +185,7 @@ export
         void Free(const AssetHandle& handle) {
             AssetInfo& asset = activeAssets[handle.slot];
             Redirect(handle, nullptr);
-            Deallocate(asset.resourceAddress);
+            Deallocate(asset.resourceAddress.load(std::memory_order_relaxed));
             asset.resourceAddress = nullptr;
 
             freeAssetSlots.push_back(handle.slot);
@@ -192,57 +193,24 @@ export
 
     protected:
         void Update() override {
-            // TODO: FinishLoadTasks()
+            FinishLoadTasks();
-            for (AssetLoadingEntry& entry : loadingEntries) {
-                if (entry.fileOwner->PollReady(*entry.file)) {
-                    AssetInfo& asset = activeAssets[entry.handle.slot];
-
-                    switch (asset.type) {
-                    case ee_assets::MAP_MESH:
-                        FinishLoadCapnprotoDeserialize(entry, mapMeshesStorage);
-                        break;
-                    case ee_assets::SAVE:
-                        FinishLoadCapnprotoDeserialize(entry, savesStorage);
-                        break;
-
-                    case ee_assets::SHADER:
-                        FinishLoadConstruct(entry, shadersStorage);
-                        break;
-                    case ee_assets::MATERIAL:
-                        FinishLoadCapnprotoDeserialize(entry, materialsStorage);
-                        break;
-                    case ee_assets::IMAGE:
-                        break;
-
-                    case ee_assets::BINARY:
-                    default:
-                        FinishLoadConstruct(entry, binaryStorage);
-                        break;
-                    }
-
-                    entry.fileOwner->Close(*entry.file);
-#ifndef NDEBUG
-                    LOG(INFO, "Asset loaded: [%hs]", entry.file->debugPath.generic_u8string().c_str());
-#endif
-                    entry.file = nullptr;
-                    asset.isReady.store(true);
-                }
-            }
-
-            std::erase_if(loadingEntries, [](const AssetLoadingEntry& entry) {
-                return entry.file == nullptr;
-            });
         }
 
         void Release() override {
             for (const AssetInfo& asset : activeAssets) {
-                //TODO: LEAK! Free resource
+                if (
-                if (asset.resourceAddress->resource.load() != nullptr) {
+                    AssetAddress* address = asset.resourceAddress.load(std::memory_order_relaxed);
-                    void *resource = asset.resourceAddress->resource.exchange(nullptr, std::memory_order_acq_rel);
+                    address != nullptr
-                    //if (resource)
+                ) {
-                    //Deallocate(resource);
+                    if (
+                        void *resource = address->resource.exchange(
+                            nullptr,
+                            std::memory_order_acq_rel
+                        )
+                    )
+                        Deallocate(resource);
+                    Deallocate(address);
                 }
-                Deallocate(asset.resourceAddress);
             }
             activeAssets.clear();
 
@@ -289,7 +257,7 @@ export
             std::size_t assetPos = 0;
             if (freeAssetSlots.size() > 0) {
                 assetPos = freeAssetSlots.back();
-                Deallocate(activeAssets[assetPos].resourceAddress);
+                Deallocate(activeAssets[assetPos].resourceAddress.load(std::memory_order_relaxed));
                 activeAssets[assetPos] = std::move(asset);
                 freeAssetSlots.pop_back();
             }
@@ -342,11 +310,57 @@ export
                 return;
 
             if (command.out != nullptr)
-                command.out->store(activeAssets[command.target.slot].resourceAddress->resource);
+                command.out->store(
+                    activeAssets[command.target.slot].resourceAddress.load(std::memory_order_relaxed)->resource
+                );
 
             command.type = AssetStreamCommand::COMPLETED;
         }
 
+        void FinishLoadTasks() {
+            diskSystem->PollLoads();
+
+            for (AssetLoadingEntry& entry : loadingEntries) {
+                if (entry.file->isCompleted) {
+                    AssetInfo& asset = activeAssets[entry.handle.slot];
+
+                    switch (asset.type) {
+                        case ee_assets::MAP_MESH:
+                            FinishLoadCapnprotoDeserialize(entry, mapMeshesStorage);
+                            break;
+                        case ee_assets::SAVE:
+                            FinishLoadCapnprotoDeserialize(entry, savesStorage);
+                            break;
+
+                        case ee_assets::SHADER:
+                            FinishLoadConstruct(entry, shadersStorage);
+                            break;
+                        case ee_assets::MATERIAL:
+                            FinishLoadCapnprotoDeserialize(entry, materialsStorage);
+                            break;
+                        case ee_assets::IMAGE:
+                            break;
+
+                        case ee_assets::BINARY:
+                        default:
+                            FinishLoadConstruct(entry, binaryStorage);
+                            break;
+                    }
+
+                    entry.fileOwner->Close(*entry.file);
+#ifndef NDEBUG
+                    LOG(INFO, "Asset loaded: [%hs]", entry.file->debugPath.generic_u8string().c_str());
+#endif
+                    entry.file = nullptr;
+                    asset.isReady.store(true);
+                }
+            }
+
+            std::erase_if(loadingEntries, [](const AssetLoadingEntry& entry) {
+                return entry.file == nullptr;
+            });
+        }
+
         template <ee_assets::Asset T>
         void FinishLoadConstruct(AssetLoadingEntry& loaded, std::vector<T*>& storage) {
             T *resource = Allocate<T>(loaded.file->data, loaded.file->size);

engine/modules/FileSystem.cppm

@@ -15,19 +15,30 @@ export
         std::uint8_t *data = nullptr;
         std::uint8_t *buffer = nullptr;
 
+        bool isCompleted = false;
+        bool isClosing = false;
+
 #ifndef NDEBUG
         std::filesystem::path debugPath;
 #endif
 
 #ifdef _WIN32
         HANDLE win32Handle = INVALID_HANDLE_VALUE;
-        OVERLAPPED overlapped;
 #endif
 
 #ifdef __linux__
 #endif
     };
 
+#ifdef _WIN32
+    struct IOContext {
+        OVERLAPPED overlapped;
+        FileHandle *file;
+        std::size_t currentOffset;
+    };
+#endif
+
+
     class IFileSystem {
     public:
         static constexpr std::uint32_t FILE_OPEN_NONE = 0;
@@ -41,7 +52,7 @@ export
 
         virtual void ReadAsync(FileHandle& file, std::uint32_t bufferSize, std::uint32_t flags) = 0;
 
-        virtual bool PollReady(FileHandle& file) = 0;
+        virtual void PollLoads() = 0;
 
         virtual void Close(FileHandle& file) = 0;
 
@@ -55,12 +66,26 @@ export
 
     class NativeFileSystem final : public IFileSystem {
     public:
+        NativeFileSystem();
+        ~NativeFileSystem() override;
+
         FileHandle& Open(const std::filesystem::path &path, std::uint32_t flags) override;
 
         void ReadAsync(FileHandle &file, std::uint32_t bufferSize, std::uint32_t flags) override;
 
-        bool PollReady(FileHandle& file) override;
+        void PollLoads() override;
 
         void Close(FileHandle &file) override;
+
+    private:
+
+#ifdef _WIN32
+        HANDLE iocpHandle = INVALID_HANDLE_VALUE;
+
+        bool HandleChunk(IOContext *context, DWORD bytesTransferred, bool success);
+        void IssueRead(IOContext *context);
+#endif
+
+        void FreeFile(FileHandle &file);
     };
 }

engine/src/FileSystem.cpp

@@ -5,8 +5,6 @@ module;
 module FileSystem;
 import utils;
 
-//TOO: REWORK WITH IOCP
-
 IFileSystem::~IFileSystem() {
     for (FileHandle *handle : activeFiles)
         Deallocate(handle);
@@ -16,6 +14,27 @@ IFileSystem::~IFileSystem() {
 
 #ifdef _WIN32
 
+NativeFileSystem::NativeFileSystem() {
+    iocpHandle = CreateIoCompletionPort(
+        INVALID_HANDLE_VALUE,
+        nullptr,
+        0,
+        0
+    );
+    if (iocpHandle == nullptr) {
+        LOG(CRITICAL, "Failed to create IOCP. Error: %d", GetLastError());
+    }
+}
+
+NativeFileSystem::~NativeFileSystem() {
+    // I have no idea why ReSharper think that this condition is always false
+    // ReSharper disable once CppDFAConstantConditions
+    if (iocpHandle != INVALID_HANDLE_VALUE && iocpHandle != nullptr) {
+        // ReSharper disable once CppDFAUnreachableCode
+        CloseHandle(iocpHandle);
+    }
+}
+
 FileHandle& NativeFileSystem::Open(const std::filesystem::path& path, const std::uint32_t flags) {
     FileHandle *result = Allocate<FileHandle>();
 
@@ -30,14 +49,14 @@ FileHandle& NativeFileSystem::Open(const std::filesystem::path& path, const std:
     if (flags & FILE_OPEN_WRITE)
         createFileMode |= GENERIC_WRITE;
 
-    HANDLE file = CreateFileW(
+    const HANDLE file = CreateFileW(
         path.c_str(),
         createFileMode,
         NULL,
-        NULL,
+        nullptr,
         OPEN_EXISTING,
         FILE_FLAG_OVERLAPPED,
-        NULL
+        nullptr
     );
 
     if (file == INVALID_HANDLE_VALUE) {
@@ -47,12 +66,23 @@ FileHandle& NativeFileSystem::Open(const std::filesystem::path& path, const std:
     }
 
     result->win32Handle = file;
-    result->size = GetFileSize(file, NULL);
+    result->size = GetFileSize(file, nullptr);
     if (result->size == INVALID_FILE_SIZE)
         LOG(ERROR, "Failed to get size of file %s. Error: %d", path.c_str(), GetLastError());
 
+    if (
+        CreateIoCompletionPort(
+            file,
+            iocpHandle,
+            NULL,
+            NULL
+        ) == nullptr
+    ) {
+        LOG(ERROR, "Failed to associate file with IOCP. Error: %d", GetLastError());
+    }
+
     if (freeFileSlots.size() > 0) {
-        std::size_t pos = freeFileSlots.back();
+        const std::size_t pos = freeFileSlots.back();
         result->id = pos;
         activeFiles[pos] = result;
         freeFileSlots.pop_back();
@@ -66,73 +96,140 @@ FileHandle& NativeFileSystem::Open(const std::filesystem::path& path, const std:
 }
 
 void NativeFileSystem::ReadAsync(FileHandle& file, const std::uint32_t bufferSize, const std::uint32_t flags) {
-    file.overlapped = {
+    file.isCompleted = false;
-        .hEvent = CreateEvent(nullptr, TRUE, FALSE, nullptr)
+    file.isClosing = false;
-    };
-    file.buffer = AllocateArray<std::uint8_t>(bufferSize);
     file.bufferSize = bufferSize;
+    file.buffer = AllocateArray<std::uint8_t>(file.bufferSize);
 
-    BOOL isSync = ReadFile(
+    IOContext *context = Allocate<IOContext>();
-        file.win32Handle,
+    context->file = &file;
-        file.buffer,
+    context->currentOffset = 0;
-        bufferSize,
+    context->overlapped = {};
-        NULL,
-        &file.overlapped
-    );
 
-    if (isSync || GetLastError() != ERROR_IO_PENDING)
+    if (!ReadFile(file.win32Handle, file.buffer, bufferSize, nullptr, &context->overlapped)) {
-        LOG(ERROR, "File reading is synchronious while asynchronious is awaited");
+        if (DWORD error = GetLastError(); error != ERROR_IO_PENDING) {
+#ifndef NDEBUG
+            LOG(ERROR, "Failed to start reading file [%s]. Error: %d", file.debugPath.c_str(), error);
+#else
+            LOG(ERROR, "Failed to start reading file [%d]. Error: %d", file.id, error);
+#endif
+            HandleChunk(context, 0, false);
+        }
+    }
 }
 
-bool NativeFileSystem::PollReady(FileHandle& file) {
+void NativeFileSystem::PollLoads() {
-    DWORD status = WaitForSingleObject(file.overlapped.hEvent, 0);
+    DWORD bytesTransferred = 0;
+    OVERLAPPED *overlapped = nullptr;
+    ULONG_PTR ck = 999;
 
-    switch (status) {
+    while (true) {
-    case WAIT_OBJECT_0:
+        const bool success = GetQueuedCompletionStatus(
-    {
+            iocpHandle,
-        if (file.size == file.bufferSize) {
+            &bytesTransferred,
+            &ck,
+            &overlapped,
+            0
+        );
+
+        if (!overlapped) { // No more ready files
+            if (DWORD error = GetLastError(); !success && error != WAIT_TIMEOUT) {
+                LOG(ERROR, "Can not get files loading status. Error: %d", error);
+            }
+            break;
+        }
+
+        if (
+            IOContext *context = reinterpret_cast<IOContext*>(overlapped);
+            HandleChunk(context, bytesTransferred, success)
+        ) {
+            IssueRead(context);
+        }
+    }
+}
+
+void NativeFileSystem::Close(FileHandle& file) {
+    if (file.win32Handle != INVALID_HANDLE_VALUE) {
+        CancelIoEx(file.win32Handle, nullptr);
+
+        if (file.isCompleted) {
+            FreeFile(file);
+        }
+        else {
+            file.isClosing = true;
+        }
+    }
+}
+
+bool NativeFileSystem::HandleChunk(IOContext *context, const DWORD bytesTransferred, const bool success) {
+    FileHandle &file = *context->file;
+
+    if (!success) {
+        if (const DWORD error = GetLastError(); error != ERROR_OPERATION_ABORTED) {
+#ifndef NDEBUG
+            LOG(ERROR, "IOCP read error [%s]. Error: %d", file.debugPath.c_str(), error);
+#else
+            LOG(ERROR, "IOCP read error [%d]. Error: %d", file.id, error);
+#endif
+        }
+
+        if (file.isClosing) FreeFile(file);
+
+        Deallocate(context);
+        file.isCompleted = true;
+        return false;
+    }
+
+    if (file.data == nullptr) { // Initialize for first chunk
+        if (file.size <= file.bufferSize) { // File of one chunk, just target resource
             file.data = file.buffer;
             file.buffer = nullptr;
         }
         else {
-            /* TODO: reserve size for data, shift and move */
+            file.data = AllocateArray<std::uint8_t>(file.size); // Allocate memory for all chunks
+        }
     }
 
-        break;
+    if (bytesTransferred == 0) { // Case of unexpected EOF
-    }
+        Deallocate(context);
-
+        if (file.isClosing) FreeFile(file);
-    case WAIT_ABANDONED:
+        file.isCompleted = true;
-    case WAIT_TIMEOUT:
-        return false;
-
-    default:
-    {
-        LOG(CRITICAL, "Failed to get file status. Error: %d", GetLastError());
         return false;
     }
+
+    if (file.buffer != nullptr) {
+        std::ranges::copy(file.buffer, file.buffer + bytesTransferred, file.data + context->currentOffset);
+    }
+    context->currentOffset += bytesTransferred;
+
+    if (context->currentOffset >= file.size) { // Finish reading of completed file
+        Deallocate(context);
+        if (file.isClosing) FreeFile(file);
+        file.isCompleted = true;
+        return false;
     }
 
     return true;
 }
 
-void NativeFileSystem::Close(FileHandle& file) {
+void NativeFileSystem::IssueRead(IOContext *context) {
-    if (file.overlapped.hEvent)
+    // 64-bit offset is seperated for WinAPI
-        CloseHandle(file.overlapped.hEvent);
+    context->overlapped.Offset = static_cast<DWORD>(context->currentOffset);
+    context->overlapped.OffsetHigh = static_cast<DWORD>(context->currentOffset >> 32);
 
-    if (file.buffer != nullptr) {
+    if (
-        DeallocateArray(file.buffer);
+        !ReadFile(
-        file.buffer = nullptr;
+            context->file->win32Handle,
+            context->file->buffer,
+            context->file->bufferSize,
+            nullptr,
+            &context->overlapped
+        )
+    ) {
+        if (GetLastError() != ERROR_IO_PENDING) {
+            HandleChunk(context, 0, false);
         }
-    if (file.data != nullptr) {
-        DeallocateArray(file.data);
-        file.data = nullptr;
     }
-
-    if (file.win32Handle != INVALID_HANDLE_VALUE) {
-        CloseHandle(file.win32Handle);
-        file.win32Handle = INVALID_HANDLE_VALUE;
-    }
-    freeFileSlots.push_back(file.id);
 }
 
 #endif
@@ -141,3 +238,17 @@ void NativeFileSystem::Close(FileHandle& file) {
 
 
 #endif
+
+void NativeFileSystem::FreeFile(FileHandle &file) {
+    if (file.buffer != nullptr) DeallocateArray(file.buffer);
+    if (file.data != nullptr) DeallocateArray(file.data);
+    file.buffer = nullptr;
+    file.data = nullptr;
+
+#ifdef _WIN32
+    if (file.win32Handle != INVALID_HANDLE_VALUE) CloseHandle(file.win32Handle);
+    file.win32Handle = INVALID_HANDLE_VALUE;
+#endif
+
+    freeFileSlots.push_back(file.id);
+}