From 2341d1ee5007a952768ccb726e4a4f3654ae96c7 Mon Sep 17 00:00:00 2001 From: Rangi42 Date: Mon, 28 Jul 2025 15:15:40 -0400 Subject: [PATCH] Replace platform-specific `mmap` with reading the entire .asm file --- include/asm/lexer.hpp | 4 +- src/asm/lexer.cpp | 121 +++++++++++------------------------------- 2 files changed, 32 insertions(+), 93 deletions(-) diff --git a/include/asm/lexer.hpp b/include/asm/lexer.hpp index 0e58fd50..0d605026 100644 --- a/include/asm/lexer.hpp +++ b/include/asm/lexer.hpp @@ -13,8 +13,8 @@ #include "platform.hpp" // SSIZE_MAX -// This value is a compromise between `LexerState` allocation performance when `mmap` works, and -// buffering performance when it doesn't/can't (e.g. when piping a file into RGBASM). +// This value is a compromise between `LexerState` allocation performance when reading the entire +// file works, and buffering performance when it doesn't (e.g. when piping a file into RGBASM). static constexpr size_t LEXER_BUF_SIZE = 64; // The buffer needs to be large enough for the maximum `lexerState->peek()` lookahead distance static_assert(LEXER_BUF_SIZE > 1, "Lexer buffer size is too small"); diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 9224031b..6bacef22 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -32,72 +33,6 @@ // Include this last so it gets all type & constant definitions #include "parser.hpp" // For token definitions, generated from parser.y -// Neither MSVC nor MinGW provide `mmap` -#if defined(_MSC_VER) || defined(__MINGW32__) -// clang-format off: maintain `include` order - #define WIN32_LEAN_AND_MEAN // Include less from `windows.h` - #include // target architecture -// clang-format on - #include // CreateFileA - #include // CloseHandle - #include // MapViewOfFile - #include // CreateFileMappingA - -static char *mapFile(int fd, std::string const &path, size_t) { - void *mappingAddr = nullptr; - if (HANDLE file = CreateFileA( - path.c_str(), - GENERIC_READ, - FILE_SHARE_READ, - nullptr, - OPEN_EXISTING, - FILE_FLAG_POSIX_SEMANTICS | FILE_FLAG_RANDOM_ACCESS, - nullptr - ); - file != INVALID_HANDLE_VALUE) { - if (HANDLE mappingObj = CreateFileMappingA(file, nullptr, PAGE_READONLY, 0, 0, nullptr); - mappingObj != INVALID_HANDLE_VALUE) { - mappingAddr = MapViewOfFile(mappingObj, FILE_MAP_READ, 0, 0, 0); - CloseHandle(mappingObj); - } - CloseHandle(file); - } - return static_cast(mappingAddr); -} - -struct FileUnmapDeleter { - FileUnmapDeleter(size_t) {} - - void operator()(char *mappingAddr) { UnmapViewOfFile(mappingAddr); } -}; - -#else // defined(_MSC_VER) || defined(__MINGW32__) - #include - -static char *mapFile(int fd, std::string const &path, size_t size) { - void *mappingAddr = mmap(nullptr, size, PROT_READ, MAP_PRIVATE, fd, 0); - // LCOV_EXCL_START - if (mappingAddr == MAP_FAILED && errno == ENOTSUP) { - // The implementation may not support MAP_PRIVATE; try again with MAP_SHARED - // instead, offering, I believe, weaker guarantees about external modifications to - // the file while reading it. That's still better than not opening it at all, though. - verbosePrint("mmap(%s, MAP_PRIVATE) failed, retrying with MAP_SHARED\n", path.c_str()); - mappingAddr = mmap(nullptr, size, PROT_READ, MAP_SHARED, fd, 0); - } - // LCOV_EXCL_STOP - return mappingAddr != MAP_FAILED ? static_cast(mappingAddr) : nullptr; -} - -struct FileUnmapDeleter { - size_t mappingSize; - - FileUnmapDeleter(size_t mappingSize_) : mappingSize(mappingSize_) {} - - void operator()(char *mappingAddr) { munmap(mappingAddr, mappingSize); } -}; - -#endif // !( defined(_MSC_VER) || defined(__MINGW32__) ) - using namespace std::literals; // Bison 3.6 changed token "types" to "kinds"; cast to int for simple compatibility @@ -406,39 +341,43 @@ void LexerState::setFileAsNextState(std::string const &filePath, bool updateStat } path = filePath; - int fd = open(path.c_str(), O_RDONLY); - if (fd < 0) { - // LCOV_EXCL_START - fatal("Failed to open file \"%s\": %s", path.c_str(), strerror(errno)); - // LCOV_EXCL_STOP - } - - bool isMmapped = false; - if (size_t size = static_cast(statBuf.st_size); statBuf.st_size > 0) { - // Try using `mmap` for better performance - if (char *mappingAddr = mapFile(fd, path, size); mappingAddr != nullptr) { - close(fd); - content.emplace( - std::shared_ptr(mappingAddr, FileUnmapDeleter(size)), size - ); - verbosePrint("File \"%s\" is mmap()ped\n", path.c_str()); // LCOV_EXCL_LINE - isMmapped = true; - } - } + // Read the entire file for better performance + // Ideally we'd use C++20 `auto ptr = std::make_shared(size)`, + // but it has insufficient compiler support + auto ptr = std::shared_ptr(new char[size]); - if (!isMmapped) { - // Sometimes mmap() fails or isn't available, so have a fallback - content.emplace(fd); + if (std::ifstream fs(path, std::ios::binary); !fs) { + // LCOV_EXCL_START + fatal("Failed to open file \"%s\": %s", path.c_str(), strerror(errno)); + // LCOV_EXCL_STOP + } else if (!fs.read(ptr.get(), size)) { + // LCOV_EXCL_START + fatal("Failed to read file \"%s\": %s", path.c_str(), strerror(errno)); + // LCOV_EXCL_STOP + } + content.emplace(ptr, size); + + verbosePrint("File \"%s\" is fully read\n", path.c_str()); // LCOV_EXCL_LINE + } else { // LCOV_EXCL_START if (statBuf.st_size == 0) { verbosePrint("File \"%s\" is empty\n", path.c_str()); } else { - verbosePrint( - "File \"%s\" is opened; errno reports: %s\n", path.c_str(), strerror(errno) - ); + verbosePrint("Failed to stat file \"%s\": %s\n", path.c_str(), strerror(errno)); } // LCOV_EXCL_STOP + + // Have a fallback if reading the file failed + int fd = open(path.c_str(), O_RDONLY); + if (fd < 0) { + // LCOV_EXCL_START + fatal("Failed to open file \"%s\": %s", path.c_str(), strerror(errno)); + // LCOV_EXCL_STOP + } + content.emplace(fd); + + verbosePrint("File \"%s\" is opened\n", path.c_str()); // LCOV_EXCL_LINE } }