From 545aa44dfe2eda3e18dcbe5fa5ccaf29e8590302 Mon Sep 17 00:00:00 2001 From: Benjamin Winger Date: Tue, 12 Nov 2024 16:05:00 -0500 Subject: [PATCH 1/2] Implement malloc-based version of the BufferManager for WASM Pages are stored in the page state structure; optimistic reads work as normal, except we need to handle the page pointer possibly being null --- .../storage/buffer_manager/buffer_manager.h | 22 ++++++++++-- .../storage/buffer_manager/page_state.h | 25 +++++++++++++- src/storage/buffer_manager/buffer_manager.cpp | 34 ++++++++++++++----- 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/include/storage/buffer_manager/buffer_manager.h b/src/include/storage/buffer_manager/buffer_manager.h index f6dd71c602c..a1ea8a785d4 100644 --- a/src/include/storage/buffer_manager/buffer_manager.h +++ b/src/include/storage/buffer_manager/buffer_manager.h @@ -7,6 +7,8 @@ #include #include "common/types/types.h" +#include "storage/buffer_manager/memory_manager.h" +#include "storage/buffer_manager/page_state.h" #include "storage/enums/page_read_policy.h" #include "storage/file_handle.h" @@ -222,10 +224,19 @@ class BufferManager { // The function assumes that the requested page is already pinned. void unpin(FileHandle& fileHandle, common::page_idx_t pageIdx); uint8_t* getFrame(FileHandle& fileHandle, common::page_idx_t pageIdx) const { +#if BM_MALLOC + return fileHandle.getPageState(pageIdx)->getPage(); +#else return vmRegions[fileHandle.getPageSizeClass()]->getFrame(fileHandle.getFrameIdx(pageIdx)); +#endif } - common::frame_group_idx_t addNewFrameGroup(common::PageSizeClass pageSizeClass) { + common::frame_group_idx_t addNewFrameGroup( + common::PageSizeClass pageSizeClass [[maybe_unused]]) { +#if BM_MALLOC + return 0; +#else return vmRegions[pageSizeClass]->addNewFrameGroup(); +#endif } void removePageFromFrameIfNecessary(FileHandle& fileHandle, common::page_idx_t pageIdx); @@ -242,8 +253,13 @@ class BufferManager { uint64_t freeUsedMemory(uint64_t size); - void releaseFrameForPage(FileHandle& fileHandle, common::page_idx_t pageIdx) { + void releaseFrameForPage(FileHandle& fileHandle [[maybe_unused]], + common::page_idx_t pageIdx [[maybe_unused]]) { +#if BM_MALLOC + // Page is freed instead in PageState::resetToEvicted +#else vmRegions[fileHandle.getPageSizeClass()]->releaseFrame(fileHandle.getFrameIdx(pageIdx)); +#endif } uint64_t evictPages(); @@ -257,7 +273,7 @@ class BufferManager { std::atomic nonEvictableMemory; // Each VMRegion corresponds to a virtual memory region of a specific page size. Currently, we // hold two sizes of REGULAR_PAGE and TEMP_PAGE. - std::vector> vmRegions; + std::array, 2> vmRegions; std::vector> fileHandles; std::unique_ptr spiller; common::VirtualFileSystem* vfs; diff --git a/src/include/storage/buffer_manager/page_state.h b/src/include/storage/buffer_manager/page_state.h index ac48ffa4dcf..5f953e0f854 100644 --- a/src/include/storage/buffer_manager/page_state.h +++ b/src/include/storage/buffer_manager/page_state.h @@ -4,6 +4,13 @@ #include "common/assert.h" +// Alternative variant of the buffer manager which doesn't rely on MADV_DONTNEED (on Unix) for +// evicting pages (which is unavailable in Webassembly runtimes) +#define BM_MALLOC __WASM__ +#if BM_MALLOC +#include +#endif + namespace kuzu { namespace storage { @@ -74,12 +81,28 @@ class PageState { bool isDirty() const { return stateAndVersion & DIRTY_MASK; } uint64_t getStateAndVersion() const { return stateAndVersion.load(); } - void resetToEvicted() { stateAndVersion.store(EVICTED << NUM_BITS_TO_SHIFT_FOR_STATE); } + void resetToEvicted() { + stateAndVersion.store(EVICTED << NUM_BITS_TO_SHIFT_FOR_STATE); +#if BM_MALLOC + page.reset(); +#endif + } + +#if BM_MALLOC + uint8_t* getPage() const { return page.get(); } + uint8_t* allocatePage(uint64_t pageSize) { + page = std::make_unique(pageSize); + return page.get(); + } +#endif private: // Highest 1 bit is dirty bit, and the rest are page state and version bits. // In the rest bits, the lowest 1 byte is state, and the rest are version. std::atomic stateAndVersion; +#if BM_MALLOC + std::unique_ptr page; +#endif }; } // namespace storage diff --git a/src/storage/buffer_manager/buffer_manager.cpp b/src/storage/buffer_manager/buffer_manager.cpp index 8c23e05f929..627a46d2db8 100644 --- a/src/storage/buffer_manager/buffer_manager.cpp +++ b/src/storage/buffer_manager/buffer_manager.cpp @@ -84,9 +84,10 @@ BufferManager::BufferManager(const std::string& databasePath, const std::string& : bufferPoolSize{bufferPoolSize}, evictionQueue{bufferPoolSize / KUZU_PAGE_SIZE}, usedMemory{evictionQueue.getCapacity() * sizeof(EvictionCandidate)}, vfs{vfs} { verifySizeParams(bufferPoolSize, maxDBSize); - vmRegions.resize(2); +#if !BM_MALLOC vmRegions[0] = std::make_unique(REGULAR_PAGE, maxDBSize); vmRegions[1] = std::make_unique(TEMP_PAGE, bufferPoolSize); +#endif // TODO(bmwinger): It may be better to spill to disk in a different location for remote file // systems, or even in general. @@ -141,7 +142,12 @@ uint8_t* BufferManager::pin(FileHandle& fileHandle, page_idx_t pageIdx, throw BufferManagerException( "Eviction queue is full! This should be impossible."); } +#if BM_MALLOC + KU_ASSERT(pageState->getPage()); + return pageState->getPage(); +#else return getFrame(fileHandle, pageIdx); +#endif } } break; case PageState::UNLOCKED: @@ -193,12 +199,19 @@ void handleAccessViolation(unsigned int exceptionCode, PEXCEPTION_POINTERS excep // Returns true if the function completes successfully inline bool try_func(const std::function& func, uint8_t* frame, - const std::vector>& vmRegions, PageSizeClass pageSizeClass) { -#if defined(_WIN32) + const std::array, 2>& vmRegions [[maybe_unused]], + PageSizeClass pageSizeClass [[maybe_unused]]) { +#if BM_MALLOC + if (frame == nullptr) { + return false; + } +#endif + +#if defined(_WIN32) && !BM_MALLOC try { #endif func(frame); -#if defined(_WIN32) +#if defined(_WIN32) && !BM_MALLOC } catch (AccessViolation& exc) { // If we encounter an acess violation within the VM region, // the page was decomitted by another thread @@ -209,9 +222,6 @@ inline bool try_func(const std::function& func, uint8_t* frame, throw EXCEPTION_ACCESS_VIOLATION; } } -#else - (void)pageSizeClass; - (void)vmRegions; #endif return true; } @@ -309,8 +319,7 @@ bool BufferManager::claimAFrame(FileHandle& fileHandle, page_idx_t pageIdx, if (!reserve(pageSizeToClaim)) { return false; } -#ifdef _WIN32 - // We need to commit memory explicitly on Windows. +#if _WIN32 && !BM_MALLOC // Committing in this context means reserving physical memory/page file space for a segment of // virtual memory. On Linux/Unix this is automatic when you write to the memory address. auto result = @@ -408,9 +417,16 @@ void BufferManager::cachePageIntoFrame(FileHandle& fileHandle, page_idx_t pageId PageReadPolicy pageReadPolicy) { auto pageState = fileHandle.getPageState(pageIdx); pageState->clearDirty(); +#if BM_MALLOC + pageState->allocatePage(fileHandle.getPageSize()); + if (pageReadPolicy == PageReadPolicy::READ_PAGE) { + fileHandle.readPageFromDisk(pageState->getPage(), pageIdx); + } +#else if (pageReadPolicy == PageReadPolicy::READ_PAGE) { fileHandle.readPageFromDisk(getFrame(fileHandle, pageIdx), pageIdx); } +#endif } void BufferManager::removeFilePagesFromFrames(FileHandle& fileHandle) { From 6d30abf0486b12038fdaf39bae5370271c8c1ac0 Mon Sep 17 00:00:00 2001 From: Benjamin Winger Date: Wed, 13 Nov 2024 18:27:13 -0500 Subject: [PATCH 2/2] Lower default buffer pool size for tests Should have been lowered before; with the higher limit most tests never do buffer manager evictions --- extension/json/test/doc_examples.test | 1 + extension/json/test/error.test | 1 + src/include/common/constants.h | 2 +- test/test_files/copy/copy_after_error.test | 1 + test/test_files/dml_rel/create/create_ldbc_sf01.test | 2 ++ test/test_files/dml_rel/delete/delete_ldbc_sf01.test | 1 + test/test_files/dml_rel/set/set_ldbc_sf01.test | 1 + test/test_files/issue/issue5.test | 3 ++- test/test_files/tck/match/match7.test | 1 + 9 files changed, 11 insertions(+), 2 deletions(-) diff --git a/extension/json/test/doc_examples.test b/extension/json/test/doc_examples.test index 9edcf11364a..73e371f19b1 100644 --- a/extension/json/test/doc_examples.test +++ b/extension/json/test/doc_examples.test @@ -1,4 +1,5 @@ -DATASET CSV empty +-BUFFER_POOL_SIZE 134217728 -- diff --git a/extension/json/test/error.test b/extension/json/test/error.test index 8b49a131b6b..396c1e81480 100644 --- a/extension/json/test/error.test +++ b/extension/json/test/error.test @@ -1,4 +1,5 @@ -DATASET CSV empty +-BUFFER_POOL_SIZE 268435456 -- diff --git a/src/include/common/constants.h b/src/include/common/constants.h index d62e205f95f..0db44b5b389 100644 --- a/src/include/common/constants.h +++ b/src/include/common/constants.h @@ -84,7 +84,7 @@ struct BufferPoolConstants { #else static constexpr uint64_t DEFAULT_VM_REGION_MAX_SIZE = static_cast(1) << 43; // (8TB) #endif - static constexpr uint64_t DEFAULT_BUFFER_POOL_SIZE_FOR_TESTING = 1ull << 28; // (256MB) + static constexpr uint64_t DEFAULT_BUFFER_POOL_SIZE_FOR_TESTING = 1ull << 26; // (64MB) }; struct StorageConstants { diff --git a/test/test_files/copy/copy_after_error.test b/test/test_files/copy/copy_after_error.test index ef8ab46dfbe..f8623ee46bc 100644 --- a/test/test_files/copy/copy_after_error.test +++ b/test/test_files/copy/copy_after_error.test @@ -1,4 +1,5 @@ -DATASET CSV empty +-BUFFER_POOL_SIZE 268435456 -- -CASE CopyNodeAfterError diff --git a/test/test_files/dml_rel/create/create_ldbc_sf01.test b/test/test_files/dml_rel/create/create_ldbc_sf01.test index efd9a0e25b7..571a05b3740 100644 --- a/test/test_files/dml_rel/create/create_ldbc_sf01.test +++ b/test/test_files/dml_rel/create/create_ldbc_sf01.test @@ -1,4 +1,6 @@ -DATASET CSV ldbc-sf01 +-BUFFER_POOL_SIZE 134217728 + -- -CASE CreateLikeComment diff --git a/test/test_files/dml_rel/delete/delete_ldbc_sf01.test b/test/test_files/dml_rel/delete/delete_ldbc_sf01.test index ef13ff0e7e5..adc2133f5e6 100644 --- a/test/test_files/dml_rel/delete/delete_ldbc_sf01.test +++ b/test/test_files/dml_rel/delete/delete_ldbc_sf01.test @@ -1,4 +1,5 @@ -DATASET CSV ldbc-sf01 +-BUFFER_POOL_SIZE 134217728 -- -CASE DeleteLikeComment1 diff --git a/test/test_files/dml_rel/set/set_ldbc_sf01.test b/test/test_files/dml_rel/set/set_ldbc_sf01.test index 640b2a54e24..baaf64501b9 100644 --- a/test/test_files/dml_rel/set/set_ldbc_sf01.test +++ b/test/test_files/dml_rel/set/set_ldbc_sf01.test @@ -1,4 +1,5 @@ -DATASET CSV ldbc-sf01 +-BUFFER_POOL_SIZE 134217728 -- -CASE SetLikeComment diff --git a/test/test_files/issue/issue5.test b/test/test_files/issue/issue5.test index b573cfd2d20..ef7050083aa 100644 --- a/test/test_files/issue/issue5.test +++ b/test/test_files/issue/issue5.test @@ -1,4 +1,5 @@ -DATASET CSV ldbc-sf01 +-BUFFER_POOL_SIZE 134217728 -- @@ -18,4 +19,4 @@ ---- ok -STATEMENT MATCH (a:Person)-[r:knows_with_null]->(b:Person) WHERE r.__EXTRAID IS NOT NULL RETURN COUNT(*) ---- 1 -0 \ No newline at end of file +0 diff --git a/test/test_files/tck/match/match7.test b/test/test_files/tck/match/match7.test index 3649fd5eb0d..964e3b086df 100644 --- a/test/test_files/tck/match/match7.test +++ b/test/test_files/tck/match/match7.test @@ -1,4 +1,5 @@ -DATASET CSV tck +-BUFFER_POOL_SIZE 268435456 --