Skip to content

Commit

Permalink
Deprecate VFSCacheArchiveFiles, will rely on the OS to provide caching
Browse files Browse the repository at this point in the history
Remove CBufferedArchive as part of the previous
Remove mutex lock from GetFile() in CZipArchive and CSevenZipArchive by opening up to ThreadPool::GetThreadNum() instances of archive files in a lazy manner
  • Loading branch information
lhog committed Jan 24, 2025
1 parent 3bcd0a8 commit 31828fd
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 235 deletions.
8 changes: 5 additions & 3 deletions rts/System/FileSystem/ArchiveScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -977,8 +977,10 @@ bool CArchiveScanner::GetArchiveChecksum(const std::string& archiveName, Archive
// just a file, can MT
numParallelFileReads = isOnSpinningDisk ? NUM_PARALLEL_FILE_READS_SD : ThreadPool::GetNumThreads();
} break;
case ARCHIVE_TYPE_SDZ: [[fallthrough]]; // mutex locked, not thread safe, makes no sense to throw more threads on it
case ARCHIVE_TYPE_SD7: [[fallthrough]]; // mutex locked, not thread safe, makes no sense to throw more threads on it
case ARCHIVE_TYPE_SDZ: [[fallthrough]];
case ARCHIVE_TYPE_SD7:
numParallelFileReads = ThreadPool::GetNumThreads(); // will open NumThreads parallel archives, this way GetFile() is no longer mutex locked
break;
default: // just default to 1 thread
numParallelFileReads = 1;
break;
Expand Down Expand Up @@ -1034,7 +1036,7 @@ bool CArchiveScanner::GetArchiveChecksum(const std::string& archiveName, Archive
tasks.emplace_back(ThreadPool::Enqueue(ComputeHashesTask, i));
}

const auto erasePredicate = [](decltype(tasks)::value_type item) {
const auto erasePredicate = [](decltype(tasks)::value_type& item) {
using namespace std::chrono_literals;
return item.wait_for(0us) == std::future_status::ready;
};
Expand Down
64 changes: 0 additions & 64 deletions rts/System/FileSystem/Archives/BufferedArchive.cpp

This file was deleted.

53 changes: 0 additions & 53 deletions rts/System/FileSystem/Archives/BufferedArchive.h

This file was deleted.

2 changes: 1 addition & 1 deletion rts/System/FileSystem/Archives/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ include_directories(${SPRING_MINIZIP_INCLUDE_DIR})

add_definitions(${PIC_FLAG})
add_library(archives STATIC
BufferedArchive.cpp
DirArchive.cpp
IArchive.cpp
PoolArchive.cpp
Expand All @@ -19,4 +18,5 @@ add_library(archives STATIC
target_link_libraries(archives
7zip
${SPRING_MINIZIP_LIBRARY}
Tracy::TracyClient
)
28 changes: 14 additions & 14 deletions rts/System/FileSystem/Archives/IArchive.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* @brief Abstraction of different archive types
*
* Loosely resembles STL container:
* for (unsigned int fid = 0; fid < NumFiles(); ++fid) {
* for (uint32_t fid = 0; fid < NumFiles(); ++fid) {
* //stuff
* }
*/
Expand All @@ -37,12 +37,12 @@ class IArchive
* @return The amount of files in the archive, does not change during
* lifetime
*/
virtual unsigned int NumFiles() const = 0;
virtual uint32_t NumFiles() const = 0;
/**
* Returns whether the supplied fileId is valid and available in this
* archive.
*/
inline bool IsFileId(unsigned int fileId) const {
inline bool IsFileId(uint32_t fileId) const {
return (fileId < NumFiles());
}
/**
Expand All @@ -60,39 +60,39 @@ class IArchive
* @param filePath VFS path to the file, for example "maps/myMap.smf"
* @return fileID of the file, NumFiles() if not found
*/
unsigned int FindFile(const std::string& filePath) const;
uint32_t FindFile(const std::string& filePath) const;
/**
* Fetches the content of a file by its ID.
* @param fid file ID in [0, NumFiles())
* @param buffer on success, this will be filled with the contents
* of the file
* @return true if the file was found, and its contents have been
* successfully read into buffer
* @see GetFile(unsigned int fid, std::vector<std::uint8_t>& buffer)
* @see GetFile(uint32_t fid, std::vector<std::uint8_t>& buffer)
*/
virtual bool GetFile(unsigned int fid, std::vector<std::uint8_t>& buffer) = 0;
virtual bool GetFile(uint32_t fid, std::vector<std::uint8_t>& buffer) = 0;
/**
* Fetches the content of a file by its name.
* @param name VFS path to the file, for example "maps/myMap.smf"
* @param buffer on success, this will be filled with the contents
* of the file
* @return true if the file was found, and its contents have been
* successfully read into buffer
* @see GetFile(unsigned int fid, std::vector<std::uint8_t>& buffer)
* @see GetFile(uint32_t fid, std::vector<std::uint8_t>& buffer)
*/
bool GetFile(const std::string& name, std::vector<std::uint8_t>& buffer);

std::pair<std::string, int> FileInfo(unsigned int fid) const {
std::pair<std::string, int> FileInfo(uint32_t fid) const {
std::pair<std::string, int> info;
FileInfo(fid, info.first, info.second);
return info;
}

unsigned int ExtractedSize() const {
unsigned int size = 0;
uint32_t ExtractedSize() const {
uint32_t size = 0;

// no archive should be larger than 4GB when extracted
for (unsigned int fid = 0; fid < NumFiles(); fid++) {
for (uint32_t fid = 0; fid < NumFiles(); fid++) {
size += (FileInfo(fid).second);
}

Expand All @@ -102,7 +102,7 @@ class IArchive
/**
* Fetches the name and size in bytes of a file by its ID.
*/
virtual void FileInfo(unsigned int fid, std::string& name, int& size) const = 0;
virtual void FileInfo(uint32_t fid, std::string& name, int& size) const = 0;

/**
* Returns true if the cost of reading the file is qualitatively relative
Expand All @@ -115,7 +115,7 @@ class IArchive
* Most implementations may always return true.
* @return true if cost is ~ relative to its file-size
*/
virtual bool HasLowReadingCost(unsigned int fid) const { return true; }
virtual bool HasLowReadingCost(uint32_t fid) const { return true; }

/**
* @return true if archive type can be packed solid (which is VERY slow when reading)
Expand All @@ -131,7 +131,7 @@ class IArchive
// Spring expects the contents of archives to be case-independent
// this map (which must be populated by subclass archives) is kept
// to allow converting back from lowercase to original case
spring::unordered_map<std::string, unsigned int> lcNameIndex;
spring::unordered_map<std::string, uint32_t> lcNameIndex;

protected:
/// "ExampleArchive.sdd"
Expand Down
10 changes: 6 additions & 4 deletions rts/System/FileSystem/Archives/PoolArchive.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* This file is part of the Spring engine (GPL v2 or later), see LICENSE.html */


#include "PoolArchive.h"

#include <algorithm>
Expand All @@ -13,6 +12,8 @@

#include "System/FileSystem/DataDirsAccess.h"
#include "System/FileSystem/FileSystem.h"
#include "System/Threading/SpringThreading.h"
#include "System/Misc/SpringTime.h"
#include "System/Exceptions.h"
#include "System/StringUtil.h"
#include "System/Log/ILog.h"
Expand All @@ -39,14 +40,15 @@ static uint32_t parse_uint32(uint8_t c[4])
return i;
}

static bool gz_really_read(gzFile file, voidp buf, unsigned int len)
static bool gz_really_read(gzFile file, voidp buf, uint32_t len)
{
return (gzread(file, reinterpret_cast<char*>(buf), len) == len);
}



CPoolArchive::CPoolArchive(const std::string& name): CBufferedArchive(name)
CPoolArchive::CPoolArchive(const std::string& name)
: IArchive(name)
{
memset(&dummyFileHash, 0, sizeof(dummyFileHash));

Expand Down Expand Up @@ -131,7 +133,7 @@ std::string CPoolArchive::GetPoolRootDirectory(const std::string& sdpName)
return poolRootDir;
}

int CPoolArchive::GetFileImpl(unsigned int fid, std::vector<std::uint8_t>& buffer)
bool CPoolArchive::GetFile(uint32_t fid, std::vector<std::uint8_t>& buffer)
{
assert(IsFileId(fid));

Expand Down
15 changes: 8 additions & 7 deletions rts/System/FileSystem/Archives/PoolArchive.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@

#include <zlib.h>
#include <cstring>
#include <cassert>

#include "IArchiveFactory.h"
#include "BufferedArchive.h"
#include "IArchive.h"


/**
Expand Down Expand Up @@ -72,7 +73,7 @@ class CPoolArchiveFactory : public IArchiveFactory {
*
* @author Chris Clearwater (det) <[email protected]>
*/
class CPoolArchive : public CBufferedArchive
class CPoolArchive : public IArchive
{
public:
CPoolArchive(const std::string& name);
Expand All @@ -83,7 +84,7 @@ class CPoolArchive : public CBufferedArchive
bool IsOpen() override { return isOpen; }

unsigned NumFiles() const override { return (files.size()); }
void FileInfo(unsigned int fid, std::string& name, int& size) const override {
void FileInfo(uint32_t fid, std::string& name, int& size) const override {
assert(IsFileId(fid));
name = files[fid].name;
size = files[fid].size;
Expand All @@ -95,15 +96,15 @@ class CPoolArchive : public CBufferedArchive

// pool-entry hashes are not calculated until GetFileImpl, must check JIT
if (memcmp(fd.shasum.data(), dummyFileHash.data(), sizeof(fd.shasum)) == 0)
GetFileImpl(fid, fb);
GetFile(fid, fb);

memcpy(hash, fd.shasum.data(), sha512::SHA_LEN);
return (memcmp(fd.shasum.data(), dummyFileHash.data(), sizeof(fd.shasum)) != 0);
}
static std::string GetPoolRootDirectory(const std::string& sdpName);
protected:
int GetFileImpl(unsigned int fid, std::vector<std::uint8_t>& buffer) override;
bool GetFile(uint32_t fid, std::vector<std::uint8_t>& buffer) override;

static std::string GetPoolRootDirectory(const std::string& sdpName);
private:
std::pair<uint64_t, uint64_t> GetSums() const {
std::pair<uint64_t, uint64_t> p;

Expand Down
Loading

0 comments on commit 31828fd

Please sign in to comment.