From f7bc75ae6d626ba3b6c9253cc1c96b9898e8a80e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 12 May 2018 17:10:03 -0400 Subject: [PATCH 1/2] Boot: Make BootExecutableReader's constructor take a std::vector by value This allows avoiding two copies of the executable data being created in the following scenario (using pseudocode): some_function() { std::vector data = ...; DolReader reader{data}; ... } In this scenario, if we only use the data for passing it to DolReader, then we have to perform a copy, as the constructor takes the std::vector as a constant reference -- you cannot move from a constant reference, and so we copy data into the DolReader, and perform another copy in the constructor itself when assigning the data to the m_bytes member variable. However, we can do better. Now, the following is allowable as well: some_function() { std::vector data = ...; DolReader reader{std::move(data)}; ... } and now we perform no copy at any point in the reader's construction, as we just std::move the data all the way through to m_bytes. In the case where we *do* want to keep the executable data around after constructing the reader, then we can just pass the vector without std::move-ing it, and we only perform a copy once (as we'll std::move said copy into m_bytes). Therefore, we get a more flexible interface resource-wise out of it. --- Source/Core/Core/Boot/Boot.cpp | 3 ++- Source/Core/Core/Boot/Boot.h | 2 +- Source/Core/Core/Boot/DolReader.cpp | 4 ++-- Source/Core/Core/Boot/DolReader.h | 2 +- Source/Core/Core/Boot/ElfReader.cpp | 2 +- Source/Core/Core/Boot/ElfReader.h | 2 +- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Source/Core/Core/Boot/Boot.cpp b/Source/Core/Core/Boot/Boot.cpp index 06d234f220..28e52119f9 100644 --- a/Source/Core/Core/Boot/Boot.cpp +++ b/Source/Core/Core/Boot/Boot.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -456,7 +457,7 @@ BootExecutableReader::BootExecutableReader(File::IOFile file) file.ReadBytes(m_bytes.data(), m_bytes.size()); } -BootExecutableReader::BootExecutableReader(const std::vector& bytes) : m_bytes(bytes) +BootExecutableReader::BootExecutableReader(std::vector bytes) : m_bytes(std::move(bytes)) { } diff --git a/Source/Core/Core/Boot/Boot.h b/Source/Core/Core/Boot/Boot.h index b1b5ad3c07..24e1c7ffd1 100644 --- a/Source/Core/Core/Boot/Boot.h +++ b/Source/Core/Core/Boot/Boot.h @@ -124,7 +124,7 @@ class BootExecutableReader public: explicit BootExecutableReader(const std::string& file_name); explicit BootExecutableReader(File::IOFile file); - explicit BootExecutableReader(const std::vector& buffer); + explicit BootExecutableReader(std::vector buffer); virtual ~BootExecutableReader(); virtual u32 GetEntryPoint() const = 0; diff --git a/Source/Core/Core/Boot/DolReader.cpp b/Source/Core/Core/Boot/DolReader.cpp index ef1ece315b..0fb8a4cdc8 100644 --- a/Source/Core/Core/Boot/DolReader.cpp +++ b/Source/Core/Core/Boot/DolReader.cpp @@ -14,9 +14,9 @@ #include "Common/Swap.h" #include "Core/HW/Memmap.h" -DolReader::DolReader(const std::vector& buffer) : BootExecutableReader(buffer) +DolReader::DolReader(std::vector buffer) : BootExecutableReader(std::move(buffer)) { - m_is_valid = Initialize(buffer); + m_is_valid = Initialize(m_bytes); } DolReader::DolReader(File::IOFile file) : BootExecutableReader(std::move(file)) diff --git a/Source/Core/Core/Boot/DolReader.h b/Source/Core/Core/Boot/DolReader.h index e8a05f771c..336b839496 100644 --- a/Source/Core/Core/Boot/DolReader.h +++ b/Source/Core/Core/Boot/DolReader.h @@ -20,7 +20,7 @@ class DolReader final : public BootExecutableReader public: explicit DolReader(const std::string& filename); explicit DolReader(File::IOFile file); - explicit DolReader(const std::vector& buffer); + explicit DolReader(std::vector buffer); ~DolReader(); bool IsValid() const override { return m_is_valid; } diff --git a/Source/Core/Core/Boot/ElfReader.cpp b/Source/Core/Core/Boot/ElfReader.cpp index 99faaa6fda..273ec93d4a 100644 --- a/Source/Core/Core/Boot/ElfReader.cpp +++ b/Source/Core/Core/Boot/ElfReader.cpp @@ -68,7 +68,7 @@ static void byteswapSection(Elf32_Shdr& sec) bswap(sec.sh_type); } -ElfReader::ElfReader(const std::vector& buffer) : BootExecutableReader(buffer) +ElfReader::ElfReader(std::vector buffer) : BootExecutableReader(std::move(buffer)) { Initialize(m_bytes.data()); } diff --git a/Source/Core/Core/Boot/ElfReader.h b/Source/Core/Core/Boot/ElfReader.h index 5d0cfc353a..c2d58173ba 100644 --- a/Source/Core/Core/Boot/ElfReader.h +++ b/Source/Core/Core/Boot/ElfReader.h @@ -28,7 +28,7 @@ class ElfReader final : public BootExecutableReader public: explicit ElfReader(const std::string& filename); explicit ElfReader(File::IOFile file); - explicit ElfReader(const std::vector& buffer); + explicit ElfReader(std::vector buffer); ~ElfReader(); u32 Read32(int off) const { return base32[off >> 2]; } // Quick accessors From 411fc012622cf52bab0edc9cccb04a0890fd221f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 12 May 2018 17:25:04 -0400 Subject: [PATCH 2/2] DolReader/ElfReader: Remove unnecessary FileUtil.h includes These can be replace with File.h, as the only file-related things necessary is the declaration of IOFile (which resides in File.h). --- Source/Core/Core/Boot/DolReader.cpp | 1 - Source/Core/Core/Boot/ElfReader.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/Core/Core/Boot/DolReader.cpp b/Source/Core/Core/Boot/DolReader.cpp index 0fb8a4cdc8..1fd03fd9a1 100644 --- a/Source/Core/Core/Boot/DolReader.cpp +++ b/Source/Core/Core/Boot/DolReader.cpp @@ -10,7 +10,6 @@ #include #include "Common/File.h" -#include "Common/FileUtil.h" #include "Common/Swap.h" #include "Core/HW/Memmap.h" diff --git a/Source/Core/Core/Boot/ElfReader.cpp b/Source/Core/Core/Boot/ElfReader.cpp index 273ec93d4a..a57b569d02 100644 --- a/Source/Core/Core/Boot/ElfReader.cpp +++ b/Source/Core/Core/Boot/ElfReader.cpp @@ -8,7 +8,7 @@ #include #include "Common/CommonTypes.h" -#include "Common/FileUtil.h" +#include "Common/File.h" #include "Common/Logging/Log.h" #include "Common/MsgHandler.h" #include "Common/Swap.h"