Merge pull request #11774 from liamwhite/refcount-issue

fsmitm_romfsbuild: avoid unnecessary copies of vfs pointers
This commit is contained in:
liamwhite 2023-10-17 11:49:11 -04:00 committed by GitHub
commit bd05ace08d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 80 additions and 88 deletions

View file

@ -116,11 +116,8 @@ FileSys::VirtualFile GetGameFileFromPath(const FileSys::VirtualFilesystem& vfs,
} }
} }
if (concat.empty()) { return FileSys::ConcatenatedVfsFile::MakeConcatenatedFile(dir->GetName(),
return nullptr; std::move(concat));
}
return FileSys::ConcatenatedVfsFile::MakeConcatenatedFile(concat, dir->GetName());
} }
if (Common::FS::IsDir(path)) { if (Common::FS::IsDir(path)) {

View file

@ -107,46 +107,25 @@ static u64 romfs_get_hash_table_count(u64 num_entries) {
void RomFSBuildContext::VisitDirectory(VirtualDir romfs_dir, VirtualDir ext_dir, void RomFSBuildContext::VisitDirectory(VirtualDir romfs_dir, VirtualDir ext_dir,
std::shared_ptr<RomFSBuildDirectoryContext> parent) { std::shared_ptr<RomFSBuildDirectoryContext> parent) {
std::vector<std::shared_ptr<RomFSBuildDirectoryContext>> child_dirs; for (auto& child_romfs_file : romfs_dir->GetFiles()) {
const auto name = child_romfs_file->GetName();
const auto entries = romfs_dir->GetEntries();
for (const auto& kv : entries) {
if (kv.second == VfsEntryType::Directory) {
const auto child = std::make_shared<RomFSBuildDirectoryContext>();
// Set child's path.
child->cur_path_ofs = parent->path_len + 1;
child->path_len = child->cur_path_ofs + static_cast<u32>(kv.first.size());
child->path = parent->path + "/" + kv.first;
if (ext_dir != nullptr && ext_dir->GetFile(kv.first + ".stub") != nullptr) {
continue;
}
// Sanity check on path_len
ASSERT(child->path_len < FS_MAX_PATH);
if (AddDirectory(parent, child)) {
child_dirs.push_back(child);
}
} else {
const auto child = std::make_shared<RomFSBuildFileContext>(); const auto child = std::make_shared<RomFSBuildFileContext>();
// Set child's path. // Set child's path.
child->cur_path_ofs = parent->path_len + 1; child->cur_path_ofs = parent->path_len + 1;
child->path_len = child->cur_path_ofs + static_cast<u32>(kv.first.size()); child->path_len = child->cur_path_ofs + static_cast<u32>(name.size());
child->path = parent->path + "/" + kv.first; child->path = parent->path + "/" + name;
if (ext_dir != nullptr && ext_dir->GetFile(kv.first + ".stub") != nullptr) { if (ext_dir != nullptr && ext_dir->GetFile(name + ".stub") != nullptr) {
continue; continue;
} }
// Sanity check on path_len // Sanity check on path_len
ASSERT(child->path_len < FS_MAX_PATH); ASSERT(child->path_len < FS_MAX_PATH);
child->source = romfs_dir->GetFile(kv.first); child->source = std::move(child_romfs_file);
if (ext_dir != nullptr) { if (ext_dir != nullptr) {
if (const auto ips = ext_dir->GetFile(kv.first + ".ips")) { if (const auto ips = ext_dir->GetFile(name + ".ips")) {
if (auto patched = PatchIPS(child->source, ips)) { if (auto patched = PatchIPS(child->source, ips)) {
child->source = std::move(patched); child->source = std::move(patched);
} }
@ -157,12 +136,27 @@ void RomFSBuildContext::VisitDirectory(VirtualDir romfs_dir, VirtualDir ext_dir,
AddFile(parent, child); AddFile(parent, child);
} }
for (auto& child_romfs_dir : romfs_dir->GetSubdirectories()) {
const auto name = child_romfs_dir->GetName();
const auto child = std::make_shared<RomFSBuildDirectoryContext>();
// Set child's path.
child->cur_path_ofs = parent->path_len + 1;
child->path_len = child->cur_path_ofs + static_cast<u32>(name.size());
child->path = parent->path + "/" + name;
if (ext_dir != nullptr && ext_dir->GetFile(name + ".stub") != nullptr) {
continue;
} }
for (auto& child : child_dirs) { // Sanity check on path_len
auto subdir_name = std::string_view(child->path).substr(child->cur_path_ofs); ASSERT(child->path_len < FS_MAX_PATH);
auto child_romfs_dir = romfs_dir->GetSubdirectory(subdir_name);
auto child_ext_dir = ext_dir != nullptr ? ext_dir->GetSubdirectory(subdir_name) : nullptr; if (!AddDirectory(parent, child)) {
continue;
}
auto child_ext_dir = ext_dir != nullptr ? ext_dir->GetSubdirectory(name) : nullptr;
this->VisitDirectory(child_romfs_dir, child_ext_dir, child); this->VisitDirectory(child_romfs_dir, child_ext_dir, child);
} }
} }
@ -293,7 +287,7 @@ std::multimap<u64, VirtualFile> RomFSBuildContext::Build() {
cur_entry.name_size = name_size; cur_entry.name_size = name_size;
out.emplace(cur_file->offset + ROMFS_FILEPARTITION_OFS, cur_file->source); out.emplace(cur_file->offset + ROMFS_FILEPARTITION_OFS, std::move(cur_file->source));
std::memcpy(file_table.data() + cur_file->entry_offset, &cur_entry, sizeof(RomFSFileEntry)); std::memcpy(file_table.data() + cur_file->entry_offset, &cur_entry, sizeof(RomFSFileEntry));
std::memset(file_table.data() + cur_file->entry_offset + sizeof(RomFSFileEntry), 0, std::memset(file_table.data() + cur_file->entry_offset + sizeof(RomFSFileEntry), 0,
Common::AlignUp(cur_entry.name_size, 4)); Common::AlignUp(cur_entry.name_size, 4));

View file

@ -377,16 +377,16 @@ static void ApplyLayeredFS(VirtualFile& romfs, u64 title_id, ContentRecordType t
auto romfs_dir = FindSubdirectoryCaseless(subdir, "romfs"); auto romfs_dir = FindSubdirectoryCaseless(subdir, "romfs");
if (romfs_dir != nullptr) if (romfs_dir != nullptr)
layers.push_back(std::make_shared<CachedVfsDirectory>(romfs_dir)); layers.emplace_back(std::make_shared<CachedVfsDirectory>(std::move(romfs_dir)));
auto ext_dir = FindSubdirectoryCaseless(subdir, "romfs_ext"); auto ext_dir = FindSubdirectoryCaseless(subdir, "romfs_ext");
if (ext_dir != nullptr) if (ext_dir != nullptr)
layers_ext.push_back(std::make_shared<CachedVfsDirectory>(ext_dir)); layers_ext.emplace_back(std::make_shared<CachedVfsDirectory>(std::move(ext_dir)));
if (type == ContentRecordType::HtmlDocument) { if (type == ContentRecordType::HtmlDocument) {
auto manual_dir = FindSubdirectoryCaseless(subdir, "manual_html"); auto manual_dir = FindSubdirectoryCaseless(subdir, "manual_html");
if (manual_dir != nullptr) if (manual_dir != nullptr)
layers.push_back(std::make_shared<CachedVfsDirectory>(manual_dir)); layers.emplace_back(std::make_shared<CachedVfsDirectory>(std::move(manual_dir)));
} }
} }
@ -400,7 +400,7 @@ static void ApplyLayeredFS(VirtualFile& romfs, u64 title_id, ContentRecordType t
return; return;
} }
layers.push_back(std::move(extracted)); layers.emplace_back(std::move(extracted));
auto layered = LayeredVfsDirectory::MakeLayeredDirectory(std::move(layers)); auto layered = LayeredVfsDirectory::MakeLayeredDirectory(std::move(layers));
if (layered == nullptr) { if (layered == nullptr) {

View file

@ -322,7 +322,8 @@ VirtualFile RegisteredCache::OpenFileOrDirectoryConcat(const VirtualDir& open_di
return nullptr; return nullptr;
} }
return ConcatenatedVfsFile::MakeConcatenatedFile(concat, concat.front()->GetName()); auto name = concat.front()->GetName();
return ConcatenatedVfsFile::MakeConcatenatedFile(std::move(name), std::move(concat));
} }
VirtualFile RegisteredCache::GetFileAtID(NcaID id) const { VirtualFile RegisteredCache::GetFileAtID(NcaID id) const {

View file

@ -133,7 +133,7 @@ VirtualDir ExtractRomFS(VirtualFile file, RomFSExtractionType type) {
out = out->GetSubdirectories().front(); out = out->GetSubdirectories().front();
} }
return std::make_shared<CachedVfsDirectory>(out); return std::make_shared<CachedVfsDirectory>(std::move(out));
} }
VirtualFile CreateRomFS(VirtualDir dir, VirtualDir ext) { VirtualFile CreateRomFS(VirtualDir dir, VirtualDir ext) {
@ -141,8 +141,7 @@ VirtualFile CreateRomFS(VirtualDir dir, VirtualDir ext) {
return nullptr; return nullptr;
RomFSBuildContext ctx{dir, ext}; RomFSBuildContext ctx{dir, ext};
auto file_map = ctx.Build(); return ConcatenatedVfsFile::MakeConcatenatedFile(0, dir->GetName(), ctx.Build());
return ConcatenatedVfsFile::MakeConcatenatedFile(0, file_map, dir->GetName());
} }
} // namespace FileSys } // namespace FileSys

View file

@ -6,13 +6,13 @@
namespace FileSys { namespace FileSys {
CachedVfsDirectory::CachedVfsDirectory(VirtualDir& source_dir) CachedVfsDirectory::CachedVfsDirectory(VirtualDir&& source_dir)
: name(source_dir->GetName()), parent(source_dir->GetParentDirectory()) { : name(source_dir->GetName()), parent(source_dir->GetParentDirectory()) {
for (auto& dir : source_dir->GetSubdirectories()) { for (auto& dir : source_dir->GetSubdirectories()) {
dirs.emplace(dir->GetName(), std::make_shared<CachedVfsDirectory>(dir)); dirs.emplace(dir->GetName(), std::make_shared<CachedVfsDirectory>(std::move(dir)));
} }
for (auto& file : source_dir->GetFiles()) { for (auto& file : source_dir->GetFiles()) {
files.emplace(file->GetName(), file); files.emplace(file->GetName(), std::move(file));
} }
} }

View file

@ -11,7 +11,7 @@ namespace FileSys {
class CachedVfsDirectory : public ReadOnlyVfsDirectory { class CachedVfsDirectory : public ReadOnlyVfsDirectory {
public: public:
CachedVfsDirectory(VirtualDir& source_directory); CachedVfsDirectory(VirtualDir&& source_directory);
~CachedVfsDirectory() override; ~CachedVfsDirectory() override;
VirtualFile GetFile(std::string_view file_name) const override; VirtualFile GetFile(std::string_view file_name) const override;

View file

@ -10,7 +10,7 @@
namespace FileSys { namespace FileSys {
ConcatenatedVfsFile::ConcatenatedVfsFile(ConcatenationMap&& concatenation_map_, std::string&& name_) ConcatenatedVfsFile::ConcatenatedVfsFile(std::string&& name_, ConcatenationMap&& concatenation_map_)
: concatenation_map(std::move(concatenation_map_)), name(std::move(name_)) { : concatenation_map(std::move(concatenation_map_)), name(std::move(name_)) {
DEBUG_ASSERT(this->VerifyContinuity()); DEBUG_ASSERT(this->VerifyContinuity());
} }
@ -30,8 +30,8 @@ bool ConcatenatedVfsFile::VerifyContinuity() const {
ConcatenatedVfsFile::~ConcatenatedVfsFile() = default; ConcatenatedVfsFile::~ConcatenatedVfsFile() = default;
VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(const std::vector<VirtualFile>& files, VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(std::string&& name,
std::string&& name) { std::vector<VirtualFile>&& files) {
// Fold trivial cases. // Fold trivial cases.
if (files.empty()) { if (files.empty()) {
return nullptr; return nullptr;
@ -46,20 +46,21 @@ VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(const std::vector<VirtualF
u64 last_offset = 0; u64 last_offset = 0;
for (auto& file : files) { for (auto& file : files) {
const auto size = file->GetSize();
concatenation_map.emplace_back(ConcatenationEntry{ concatenation_map.emplace_back(ConcatenationEntry{
.offset = last_offset, .offset = last_offset,
.file = file, .file = std::move(file),
}); });
last_offset += file->GetSize(); last_offset += size;
} }
return VirtualFile(new ConcatenatedVfsFile(std::move(concatenation_map), std::move(name))); return VirtualFile(new ConcatenatedVfsFile(std::move(name), std::move(concatenation_map)));
} }
VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(u8 filler_byte, VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(u8 filler_byte, std::string&& name,
const std::multimap<u64, VirtualFile>& files, std::multimap<u64, VirtualFile>&& files) {
std::string&& name) {
// Fold trivial cases. // Fold trivial cases.
if (files.empty()) { if (files.empty()) {
return nullptr; return nullptr;
@ -76,6 +77,8 @@ VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(u8 filler_byte,
// Iteration of a multimap is ordered, so offset will be strictly non-decreasing. // Iteration of a multimap is ordered, so offset will be strictly non-decreasing.
for (auto& [offset, file] : files) { for (auto& [offset, file] : files) {
const auto size = file->GetSize();
if (offset > last_offset) { if (offset > last_offset) {
concatenation_map.emplace_back(ConcatenationEntry{ concatenation_map.emplace_back(ConcatenationEntry{
.offset = last_offset, .offset = last_offset,
@ -85,13 +88,13 @@ VirtualFile ConcatenatedVfsFile::MakeConcatenatedFile(u8 filler_byte,
concatenation_map.emplace_back(ConcatenationEntry{ concatenation_map.emplace_back(ConcatenationEntry{
.offset = offset, .offset = offset,
.file = file, .file = std::move(file),
}); });
last_offset = offset + file->GetSize(); last_offset = offset + size;
} }
return VirtualFile(new ConcatenatedVfsFile(std::move(concatenation_map), std::move(name))); return VirtualFile(new ConcatenatedVfsFile(std::move(name), std::move(concatenation_map)));
} }
std::string ConcatenatedVfsFile::GetName() const { std::string ConcatenatedVfsFile::GetName() const {

View file

@ -24,22 +24,20 @@ private:
}; };
using ConcatenationMap = std::vector<ConcatenationEntry>; using ConcatenationMap = std::vector<ConcatenationEntry>;
explicit ConcatenatedVfsFile(std::vector<ConcatenationEntry>&& concatenation_map, explicit ConcatenatedVfsFile(std::string&& name,
std::string&& name); std::vector<ConcatenationEntry>&& concatenation_map);
bool VerifyContinuity() const; bool VerifyContinuity() const;
public: public:
~ConcatenatedVfsFile() override; ~ConcatenatedVfsFile() override;
/// Wrapper function to allow for more efficient handling of files.size() == 0, 1 cases. /// Wrapper function to allow for more efficient handling of files.size() == 0, 1 cases.
static VirtualFile MakeConcatenatedFile(const std::vector<VirtualFile>& files, static VirtualFile MakeConcatenatedFile(std::string&& name, std::vector<VirtualFile>&& files);
std::string&& name);
/// Convenience function that turns a map of offsets to files into a concatenated file, filling /// Convenience function that turns a map of offsets to files into a concatenated file, filling
/// gaps with a given filler byte. /// gaps with a given filler byte.
static VirtualFile MakeConcatenatedFile(u8 filler_byte, static VirtualFile MakeConcatenatedFile(u8 filler_byte, std::string&& name,
const std::multimap<u64, VirtualFile>& files, std::multimap<u64, VirtualFile>&& files);
std::string&& name);
std::string GetName() const override; std::string GetName() const override;
std::size_t GetSize() const override; std::size_t GetSize() const override;

View file

@ -38,7 +38,7 @@ VirtualDir LayeredVfsDirectory::GetDirectoryRelative(std::string_view path) cons
for (const auto& layer : dirs) { for (const auto& layer : dirs) {
auto dir = layer->GetDirectoryRelative(path); auto dir = layer->GetDirectoryRelative(path);
if (dir != nullptr) { if (dir != nullptr) {
out.push_back(std::move(dir)); out.emplace_back(std::move(dir));
} }
} }
@ -62,11 +62,11 @@ std::vector<VirtualFile> LayeredVfsDirectory::GetFiles() const {
std::set<std::string, std::less<>> out_names; std::set<std::string, std::less<>> out_names;
for (const auto& layer : dirs) { for (const auto& layer : dirs) {
for (const auto& file : layer->GetFiles()) { for (auto& file : layer->GetFiles()) {
auto file_name = file->GetName(); auto file_name = file->GetName();
if (!out_names.contains(file_name)) { if (!out_names.contains(file_name)) {
out_names.emplace(std::move(file_name)); out_names.emplace(std::move(file_name));
out.push_back(file); out.emplace_back(std::move(file));
} }
} }
} }
@ -86,7 +86,7 @@ std::vector<VirtualDir> LayeredVfsDirectory::GetSubdirectories() const {
std::vector<VirtualDir> out; std::vector<VirtualDir> out;
out.reserve(names.size()); out.reserve(names.size());
for (const auto& subdir : names) for (const auto& subdir : names)
out.push_back(GetSubdirectory(subdir)); out.emplace_back(GetSubdirectory(subdir));
return out; return out;
} }