Skip to content

Commit

Permalink
IGL: Switch FileLoader to use unique_ptr<uint8_t[]>
Browse files Browse the repository at this point in the history
Summary: Thsi diff updates IGL shell's file loader to use std::unique_ptr<uint8_t[]> instead of std::vector<uint8_t>. This avoids zeroing out the memory on allocation

Reviewed By: corporateshark

Differential Revision: D49135944

fbshipit-source-id: 518f96b5058288d4b21b0a6f35c0961a29a0440d
  • Loading branch information
Eric Griffith authored and facebook-github-bot committed Sep 15, 2023
1 parent 7341893 commit af72c2c
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 26 deletions.
8 changes: 4 additions & 4 deletions shell/shared/fileLoader/FileLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace fs = boost::filesystem;

namespace igl::shell {

std::vector<uint8_t> FileLoader::loadBinaryDataInternal(const std::string& filePath) {
FileLoader::FileData FileLoader::loadBinaryDataInternal(const std::string& filePath) {
if (IGL_UNEXPECTED(!fs::exists(filePath))) {
return {};
}
Expand All @@ -39,12 +39,12 @@ std::vector<uint8_t> FileLoader::loadBinaryDataInternal(const std::string& fileP
std::fclose(file);
};

std::vector<uint8_t> data(static_cast<size_t>(length));
if (std::fread(data.data(), 1, data.size(), file) != data.size()) {
auto data = std::make_unique<uint8_t[]>(static_cast<size_t>(length));
if (std::fread(data.get(), 1, length, file) != length) {
return {};
}

return data;
return {std::move(data), static_cast<uint32_t>(length)};
}

} // namespace igl::shell
12 changes: 9 additions & 3 deletions shell/shared/fileLoader/FileLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,23 @@
#pragma once

#include <cstdint>
#include <memory>
#include <string>
#include <vector>

namespace igl::shell {

class FileLoader {
public:
struct FileData {
std::unique_ptr<uint8_t[]> data = nullptr;
uint32_t length;
};

FileLoader() = default;
virtual ~FileLoader() = default;
virtual std::vector<uint8_t> loadBinaryData(const std::string& /* filename */) {
return std::vector<uint8_t>();
virtual FileData loadBinaryData(const std::string& /* filename */) {
return {};
}
virtual bool fileExists(const std::string& /* filename */) const {
return false;
Expand All @@ -31,7 +37,7 @@ class FileLoader {
}

protected:
std::vector<uint8_t> loadBinaryDataInternal(const std::string& filePath);
FileData loadBinaryDataInternal(const std::string& filePath);
};

} // namespace igl::shell
31 changes: 17 additions & 14 deletions shell/shared/fileLoader/android/FileLoaderAndroid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,41 @@

namespace igl::shell {

std::vector<uint8_t> FileLoaderAndroid::loadBinaryData(const std::string& fileName) {
std::vector<uint8_t> data;
FileLoader::FileData FileLoaderAndroid::loadBinaryData(const std::string& fileName) {
if (fileName.empty()) {
IGL_LOG_ERROR("Error in loadBinaryData(): empty fileName\n");
return data;
return {};
}

if (assetManager_ == nullptr) {
IGL_LOG_ERROR("Error in loadBinaryData(): Asset manager is nullptr\n");
return data;
return {};
}

// Load file
AAsset* asset = AAssetManager_open(assetManager_, fileName.c_str(), AASSET_MODE_BUFFER);
IGL_ASSERT(asset != nullptr);
if (asset == nullptr) {
if (IGL_UNEXPECTED(asset == nullptr)) {
IGL_LOG_ERROR("Error in loadBinaryData(): failed to open file %s\n", fileName.c_str());
return data;
return {};
}

off64_t length = AAsset_getLength64(asset);
if (IGL_UNEXPECTED(length > std::numeric_limits<uint32_t>::max())) {
AAsset_close(asset);
return {};
}

data.resize(AAsset_getLength(asset));
auto readSize = AAsset_read(asset, data.data(), data.size());
if (readSize != data.size()) {
IGL_LOG_ERROR("Error in loadBinaryData(): read size mismatch (%ld != %ld) in %s\n",
auto data = std::make_unique<uint8_t[]>(length);
auto readSize = AAsset_read(asset, data.get(), length);
if (IGL_UNEXPECTED(readSize != length)) {
IGL_LOG_ERROR("Error in loadBinaryData(): read size mismatch (%ld != %zu) in %s\n",
readSize,
data.size(),
length,
fileName.c_str());
IGL_ASSERT_NOT_REACHED();
}
AAsset_close(asset);

return data;
return {std::move(data), static_cast<uint32_t>(length)};
}

bool FileLoaderAndroid::fileExists(const std::string& fileName) const {
Expand Down
2 changes: 1 addition & 1 deletion shell/shared/fileLoader/android/FileLoaderAndroid.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class FileLoaderAndroid final : public FileLoader {
public:
FileLoaderAndroid() = default;
~FileLoaderAndroid() override = default;
std::vector<uint8_t> loadBinaryData(const std::string& fileName) override;
FileData loadBinaryData(const std::string& fileName) override;
bool fileExists(const std::string& fileName) const override;
std::string basePath() const override;
std::string fullPath(const std::string& fileName) const override;
Expand Down
2 changes: 1 addition & 1 deletion shell/shared/fileLoader/apple/FileLoaderApple.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FileLoaderApple final : public FileLoader {
public:
FileLoaderApple() = default;
~FileLoaderApple() override = default;
[[nodiscard]] std::vector<uint8_t> loadBinaryData(const std::string& fileName) override;
[[nodiscard]] FileData loadBinaryData(const std::string& fileName) override;
[[nodiscard]] bool fileExists(const std::string& fileName) const override;
[[nodiscard]] std::string basePath() const override;
[[nodiscard]] std::string fullPath(const std::string& fileName) const override;
Expand Down
2 changes: 1 addition & 1 deletion shell/shared/fileLoader/apple/FileLoaderApple.mm
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
}
} // namespace

std::vector<uint8_t> FileLoaderApple::loadBinaryData(const std::string& fileName) {
FileLoader::FileData FileLoaderApple::loadBinaryData(const std::string& fileName) {
return loadBinaryDataInternal(fullPath(fileName));
}

Expand Down
2 changes: 1 addition & 1 deletion shell/shared/fileLoader/win/FileLoaderWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ FileLoaderWin::FileLoaderWin() {
#endif
}

std::vector<uint8_t> FileLoaderWin::loadBinaryData(const std::string& fileName) {
FileLoader::FileData FileLoaderWin::loadBinaryData(const std::string& fileName) {
return loadBinaryDataInternal(fullPath(fileName));
}

Expand Down
2 changes: 1 addition & 1 deletion shell/shared/fileLoader/win/FileLoaderWin.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FileLoaderWin final : public FileLoader {
public:
FileLoaderWin();
~FileLoaderWin() override = default;
std::vector<uint8_t> loadBinaryData(const std::string& fileName) override;
FileData loadBinaryData(const std::string& fileName) override;
bool fileExists(const std::string& fileName) const override;
std::string basePath() const override;
std::string fullPath(const std::string& fileName) const override;
Expand Down

0 comments on commit af72c2c

Please sign in to comment.