Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use memory mapped files access #1781

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions rts/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ list(APPEND engineCommonLibraries "prd::base64")
### smmalloc
list(APPEND engineCommonLibraries "smmalloc")

### mapped files(mio)
list(APPEND engineCommonLibraries mio::mio)

### Assemble common libraries
add_subdirectory(System/Sound)
if (NO_SOUND)
Expand Down
1 change: 1 addition & 0 deletions rts/Sim/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ add_library(engineSim STATIC
target_include_directories(engineSim
PRIVATE ${SDL2_INCLUDE_DIR})
target_link_libraries(engineSim Tracy::TracyClient)
target_link_libraries(engineSim mio::mio)

if( CMAKE_COMPILER_IS_GNUCXX)
# FIXME: hack to avoid linkers to remove not referenced symbols. required because of
Expand Down
14 changes: 7 additions & 7 deletions rts/System/FileSystem/ArchiveScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -604,11 +604,11 @@ std::string CArchiveScanner::SearchMapFile(const IArchive* ar, std::string& erro

// check for smf and if the uncompression of important files is too costy
for (unsigned fid = 0; fid != ar->NumFiles(); ++fid) {
const std::pair<std::string, int>& info = ar->FileInfo(fid);
const std::string& ext = FileSystem::GetExtension(StringToLower(info.first));
const auto& [name, size] = ar->FileInfo(fid);
const auto ext = FileSystem::GetExtension(StringToLower(name));

if (ext == "smf")
return info.first;
return name;
}

return "";
Expand Down Expand Up @@ -870,9 +870,9 @@ bool CArchiveScanner::CheckCachedData(const std::string& fullName, unsigned& mod

bool CArchiveScanner::ScanArchiveLua(IArchive* ar, const std::string& fileName, ArchiveInfo& ai, std::string& err)
{
std::vector<std::uint8_t> buf;

if (!ar->GetFile(fileName, buf) || buf.empty()) {
// use the same buffer to avoid allocation costs for every invocation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this could be static to the function instead of class member?

scanArchiveBuffer.clear();
if (!ar->GetFile(fileName, scanArchiveBuffer) || scanArchiveBuffer.empty()) {
err = "Error reading " + fileName;

if (ar->GetArchiveFile().find(".sdp") != std::string::npos)
Expand All @@ -882,7 +882,7 @@ bool CArchiveScanner::ScanArchiveLua(IArchive* ar, const std::string& fileName,
}

// NB: skips LuaConstGame::PushEntries(L) since that would invoke ScanArchive again
LuaParser p(std::string((char*)(buf.data()), buf.size()), SPRING_VFS_ZIP);
LuaParser p(std::string((char*)(scanArchiveBuffer.data()), scanArchiveBuffer.size()), SPRING_VFS_ZIP);

if (!p.Execute()) {
err = "Error in " + fileName + ": " + p.GetErrorLog();
Expand Down
2 changes: 2 additions & 0 deletions rts/System/FileSystem/ArchiveScanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ class CArchiveScanner
static bool CheckCompression(const IArchive* ar, const std::string& fullName, std::string& error);

private:
std::vector<std::uint8_t> scanArchiveBuffer;

spring::unordered_map<std::string, size_t> archiveInfosIndex;
spring::unordered_map<std::string, size_t> brokenArchivesIndex;

Expand Down
1 change: 1 addition & 0 deletions rts/System/FileSystem/Archives/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ add_library(archives STATIC
)

target_link_libraries(archives
mio::mio
7zip
${SPRING_MINIZIP_LIBRARY}
)
52 changes: 38 additions & 14 deletions rts/System/FileSystem/Archives/DirArchive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#include "DirArchive.h"

#include <assert.h>
#include <fstream>
#include <filesystem>

#include <mio/mmap.hpp>

#include "System/FileSystem/DataDirsAccess.h"
#include "System/FileSystem/FileSystem.h"
Expand Down Expand Up @@ -47,18 +49,29 @@ bool CDirArchive::GetFile(unsigned int fid, std::vector<std::uint8_t>& buffer)
assert(IsFileId(fid));

const std::string rawpath = dataDirsAccess.LocateFile(dirName + searchFiles[fid]);
std::ifstream ifs(rawpath.c_str(), std::ios::in | std::ios::binary);

if (ifs.bad() || !ifs.is_open())
std::error_code ec;
std::filesystem::directory_entry entry(rawpath, ec);
if (ec)
return false;

ifs.seekg(0, std::ios_base::end);
buffer.resize(ifs.tellg());
ifs.seekg(0, std::ios_base::beg);
ifs.clear();
if (!entry.exists())
return false;

if (!buffer.empty())
ifs.read((char*)&buffer[0], buffer.size());
if (!entry.is_regular_file())
return false;

mio::ummap_source mmap(rawpath);
if (!mmap.is_open()) {
return false;
}

if (!mmap.is_mapped()) {
return false;
}

buffer.resize(mmap.size());
std::memcpy(buffer.data(), mmap.data(), mmap.size());

return true;
}
Expand All @@ -69,12 +82,23 @@ void CDirArchive::FileInfo(unsigned int fid, std::string& name, int& size) const

name = searchFiles[fid];
const std::string rawPath = dataDirsAccess.LocateFile(dirName + name);
std::ifstream ifs(rawPath.c_str(), std::ios::in | std::ios::binary);
std::error_code ec;
std::filesystem::directory_entry entry(rawPath, ec);

if (ec) {
size = 0;
return;
}

if (!ifs.bad() && ifs.is_open()) {
ifs.seekg(0, std::ios_base::end);
size = ifs.tellg();
} else {
if (!entry.exists()) {
size = 0;
return;
}

if (!entry.is_regular_file()) {
size = 0;
return;
}

size = entry.file_size();
}
120 changes: 76 additions & 44 deletions rts/System/FileSystem/FileHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <string>
#include <vector>
#include <fstream>
#include <filesystem>
#include <algorithm>
#include <cassert>
#include <cctype>
Expand Down Expand Up @@ -48,6 +49,13 @@ CFileHandler::CFileHandler(const string& fileName, const string& modes)

/******************************************************************************/

std::span<const uint8_t> CFileHandler::GetSpan() const
{
return mmap ?
std::span(mmap->data(), mmap->size()) :
std::span(fileBuffer.data(), fileBuffer.size());
}

bool CFileHandler::TryReadFromPWD(const string& fileName)
{
#ifndef TOOLS
Expand All @@ -57,31 +65,75 @@ bool CFileHandler::TryReadFromPWD(const string& fileName)
#else
const std::string fullpath(fileName);
#endif
ifs.open(fullpath.c_str(), std::ios::in | std::ios::binary);
if (ifs && !ifs.bad() && ifs.is_open()) {
ifs.seekg(0, std::ios_base::end);
fileSize = ifs.tellg();
ifs.seekg(0, std::ios_base::beg);
return true;
std::error_code ec;
std::filesystem::directory_entry entry(fileName, ec);
if (ec)
return false;

if (!entry.exists())
return false;

if (!entry.is_regular_file())
return false;

fileSize = entry.file_size();

mmap = std::make_unique<mio::ummap_source>(fileName);
if (!mmap->is_open()) {
mmap = nullptr;
return false;
}
ifs.close();
return false;

if (!mmap->is_mapped()) {
mmap = nullptr;
return false;
}

if (mmap->size() != fileSize) {
mmap = nullptr;
return false;
}

return true;
}


bool CFileHandler::TryReadFromRawFS(const string& fileName)
{
#ifndef TOOLS
const string rawpath = dataDirsAccess.LocateFile(fileName);
ifs.open(rawpath.c_str(), std::ios::in | std::ios::binary);
if (ifs && !ifs.bad() && ifs.is_open()) {
ifs.seekg(0, std::ios_base::end);
fileSize = ifs.tellg();
ifs.seekg(0, std::ios_base::beg);
return true;

std::error_code ec;
std::filesystem::directory_entry entry(rawpath, ec);
if (ec)
return false;

if (!entry.exists())
return false;

if (!entry.is_regular_file())
return false;

fileSize = entry.file_size();

mmap = std::make_unique<mio::ummap_source>(rawpath);
if (!mmap->is_open()) {
mmap = nullptr;
return false;
}

if (!mmap->is_mapped()) {
mmap = nullptr;
return false;
}

if (mmap->size() != fileSize) {
mmap = nullptr;
return false;
}

return true;
#endif
ifs.close();
return false;
Copy link
Collaborator

@sprunk sprunk Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-TOOLS path will have two unconditional returns in a row now, the second one is dead code and may confuse static analysis (or produce warnings etc)

}

Expand Down Expand Up @@ -128,7 +180,7 @@ void CFileHandler::Close()
fileSize = -1;
loadCode = -3;

ifs.close();
mmap = nullptr;
fileBuffer.clear();
}

Expand Down Expand Up @@ -168,20 +220,20 @@ bool CFileHandler::FileExists(const std::string& filePath, const std::string& mo

int CFileHandler::Read(void* buf, int length)
{
if (ifs.is_open()) {
ifs.read(static_cast<char*>(buf), length);
return ifs.gcount();
}
if (fileSize <= 0)
return 0;

if (fileBuffer.empty())
if (fileBuffer.empty() && !mmap)
return 0;

const auto span = GetSpan();

if ((length + filePos) > fileSize)
length = fileSize - filePos;

if (length > 0) {
assert(fileBuffer.size() >= (filePos + length));
memcpy(buf, &fileBuffer[filePos], length);
assert(span.size() >= (filePos + length));
std::memcpy(buf, span.data() + filePos, length);
filePos += length;
}

Expand Down Expand Up @@ -210,17 +262,6 @@ int CFileHandler::ReadString(void* buf, int length)

void CFileHandler::Seek(int length, std::ios_base::seekdir where)
{
if (ifs.is_open()) {
// Status bits must be cleared before seeking, otherwise it might fail
// in the common case of EOF
// http://en.cppreference.com/w/cpp/io/basic_istream/seekg
ifs.clear();
ifs.seekg(length, where);
return;
}
if (fileBuffer.empty())
return;

switch (where) {
case std::ios_base::beg: { filePos = length; } break;
case std::ios_base::cur: { filePos += length; } break;
Expand All @@ -231,21 +272,12 @@ void CFileHandler::Seek(int length, std::ios_base::seekdir where)

bool CFileHandler::Eof() const
{
if (ifs.is_open())
return ifs.eof();

if (!fileBuffer.empty())
return (filePos >= fileSize);

return true;
return (filePos >= fileSize);
}


int CFileHandler::GetPos()
{
if (ifs.is_open())
return ifs.tellg();

return filePos;
}

Expand Down
13 changes: 7 additions & 6 deletions rts/System/FileSystem/FileHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
#include <vector>
#include <string>
#include <fstream>
#include <memory>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be added to some .cpp file(s) instead? Looks like no new code was added below so probably doesn't need to be in the header (gets applies everywhere the .h is included even if not all .cpp files need it)

#include <span>
#include <cinttypes>
#include <mio/mmap.hpp>

#include "VFSModes.h"

Expand Down Expand Up @@ -52,18 +55,15 @@ class CFileHandler

std::vector<std::uint8_t>& GetBuffer() { return fileBuffer; }

static bool InReadDir(const std::string& path);
static bool InWriteDir(const std::string& path);

static std::vector<std::string> FindFiles(const std::string& path, const std::string& pattern);
static std::vector<std::string> DirList(const std::string& path, const std::string& pattern, const std::string& modes, bool recursive);
static std::vector<std::string> SubDirs(const std::string& path, const std::string& pattern, const std::string& modes, bool recursive);

static std::string AllowModes(const std::string& modes, const std::string& allowed);
static std::string ForbidModes(const std::string& modes, const std::string& forbidden);


protected:
std::span<const uint8_t> GetSpan() const;

virtual bool TryReadFromPWD(const std::string& fileName);
virtual bool TryReadFromRawFS(const std::string& fileName);
Expand All @@ -76,8 +76,9 @@ class CFileHandler
static bool InsertVFSDirs(std::vector<std::string>& dirSet, const std::string& path, const std::string& pattern, bool recursive, int section);

std::string fileName;
std::ifstream ifs;
std::vector<std::uint8_t> fileBuffer;
std::vector<uint8_t> fileBuffer;

std::unique_ptr<mio::ummap_source> mmap;

int filePos = 0;
int fileSize = -1;
Expand Down
Loading
Loading