Skip to content

Mark thread-safe methods of DataLoader as const. #4756

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

Merged
merged 1 commit into from
Aug 16, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
return [NSData dataWithContentsOfURL:url];
}

class DataLoaderImpl: public DataLoader {
class DataLoaderImpl final : public DataLoader {
public:
DataLoaderImpl(std::string filePath)
:data_(read_data(filePath))
{}

Result<FreeableBuffer> load(
size_t offset, size_t size, __ET_UNUSED const DataLoader::SegmentInfo& segment_info) override {
size_t offset, size_t size, __ET_UNUSED const DataLoader::SegmentInfo& segment_info) const override {
NSData *subdata = [data_ subdataWithRange:NSMakeRange(offset, size)];
return FreeableBuffer(subdata.bytes, size, nullptr);
}
Expand All @@ -42,7 +42,7 @@
}

private:
NSData *data_;
NSData * const data_;
};

using Buffer = std::vector<uint8_t>;
Expand Down
6 changes: 3 additions & 3 deletions examples/apple/coreml/executor_runner/main.mm
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,13 @@ Args parse_command_line_args(NSArray<NSString *> *args) {
return data;
}

class DataLoaderImpl: public DataLoader {
class DataLoaderImpl final : public DataLoader {
public:
DataLoaderImpl(const std::string& filePath)
:data_(read_data(filePath))
{}

Result<FreeableBuffer> load(size_t offset, size_t size, __ET_UNUSED const DataLoader::SegmentInfo& segment_info) override {
Result<FreeableBuffer> load(size_t offset, size_t size, __ET_UNUSED const DataLoader::SegmentInfo& segment_info) const override {
NSData *subdata = [data_ subdataWithRange:NSMakeRange(offset, size)];
return FreeableBuffer(subdata.bytes, size, nullptr);
}
Expand All @@ -170,7 +170,7 @@ Args parse_command_line_args(NSArray<NSString *> *args) {
}

private:
NSData *data_;
NSData * const data_;
};

using Buffer = std::vector<uint8_t>;
Expand Down
6 changes: 3 additions & 3 deletions extension/data_loader/buffer_data_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ namespace util {
* This can be used to wrap data that is directly embedded into the firmware
* image, or to wrap data that was allocated elsewhere.
*/
class BufferDataLoader : public DataLoader {
class BufferDataLoader final : public DataLoader {
public:
BufferDataLoader(const void* data, size_t size)
: data_(reinterpret_cast<const uint8_t*>(data)), size_(size) {}

__ET_NODISCARD Result<FreeableBuffer> load(
size_t offset,
size_t size,
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) override {
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) const override {
ET_CHECK_OR_RETURN_ERROR(
offset + size <= size_,
InvalidArgument,
Expand All @@ -52,7 +52,7 @@ class BufferDataLoader : public DataLoader {
size_t offset,
size_t size,
__ET_UNUSED const SegmentInfo& segment_info,
void* buffer) override {
void* buffer) const override {
ET_CHECK_OR_RETURN_ERROR(
buffer != nullptr,
InvalidArgument,
Expand Down
4 changes: 2 additions & 2 deletions extension/data_loader/file_data_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ void FreeSegment(void* context, void* data, __ET_UNUSED size_t size) {
Result<FreeableBuffer> FileDataLoader::load(
size_t offset,
size_t size,
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) {
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) const {
ET_CHECK_OR_RETURN_ERROR(
// Probably had its value moved to another instance.
fd_ >= 0,
Expand Down Expand Up @@ -209,7 +209,7 @@ __ET_NODISCARD Error FileDataLoader::load_into(
size_t offset,
size_t size,
__ET_UNUSED const SegmentInfo& segment_info,
void* buffer) {
void* buffer) const {
ET_CHECK_OR_RETURN_ERROR(
// Probably had its value moved to another instance.
fd_ >= 0,
Expand Down
22 changes: 11 additions & 11 deletions extension/data_loader/file_data_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace util {
* Note that this will keep the file open for the duration of its lifetime, to
* avoid the overhead of opening it again for every load() call.
*/
class FileDataLoader : public DataLoader {
class FileDataLoader final : public DataLoader {
public:
/**
* Creates a new FileDataLoader that wraps the named file.
Expand Down Expand Up @@ -57,26 +57,26 @@ class FileDataLoader : public DataLoader {
file_size_(rhs.file_size_),
alignment_(rhs.alignment_),
fd_(rhs.fd_) {
rhs.file_name_ = nullptr;
rhs.file_size_ = 0;
rhs.alignment_ = 0;
rhs.fd_ = -1;
const_cast<const char*&>(rhs.file_name_) = nullptr;
const_cast<size_t&>(rhs.file_size_) = 0;
const_cast<size_t&>(rhs.alignment_) = 0;
const_cast<int&>(rhs.fd_) = -1;
}

~FileDataLoader() override;

__ET_NODISCARD Result<FreeableBuffer> load(
size_t offset,
size_t size,
const DataLoader::SegmentInfo& segment_info) override;
const DataLoader::SegmentInfo& segment_info) const override;

__ET_NODISCARD Result<size_t> size() const override;

__ET_NODISCARD Error load_into(
size_t offset,
size_t size,
__ET_UNUSED const SegmentInfo& segment_info,
void* buffer) override;
void* buffer) const override;

private:
FileDataLoader(
Expand All @@ -94,10 +94,10 @@ class FileDataLoader : public DataLoader {
FileDataLoader& operator=(const FileDataLoader&) = delete;
FileDataLoader& operator=(FileDataLoader&&) = delete;

const char* file_name_; // Owned by the instance.
size_t file_size_;
size_t alignment_;
int fd_; // Owned by the instance.
const char* const file_name_; // Owned by the instance.
const size_t file_size_;
const size_t alignment_;
const int fd_; // Owned by the instance.
};

} // namespace util
Expand Down
2 changes: 1 addition & 1 deletion extension/data_loader/mmap_data_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ void MunmapSegment(void* context, void* data, size_t size) {
Result<FreeableBuffer> MmapDataLoader::load(
size_t offset,
size_t size,
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) {
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) const {
ET_CHECK_OR_RETURN_ERROR(
// Probably had its value moved to another instance.
fd_ >= 0,
Expand Down
24 changes: 12 additions & 12 deletions extension/data_loader/mmap_data_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace util {
* Note that this will keep the file open for the duration of its lifetime, to
* avoid the overhead of opening it again for every load() call.
*/
class MmapDataLoader : public DataLoader {
class MmapDataLoader final : public DataLoader {
public:
/**
* Describes how and whether to lock loaded pages with `mlock()`.
Expand Down Expand Up @@ -77,19 +77,19 @@ class MmapDataLoader : public DataLoader {
page_size_(rhs.page_size_),
fd_(rhs.fd_),
mlock_config_(rhs.mlock_config_) {
rhs.file_name_ = nullptr;
rhs.file_size_ = 0;
rhs.page_size_ = 0;
rhs.fd_ = -1;
rhs.mlock_config_ = MlockConfig::NoMlock;
const_cast<const char*&>(rhs.file_name_) = nullptr;
const_cast<size_t&>(rhs.file_size_) = 0;
const_cast<size_t&>(rhs.page_size_) = 0;
const_cast<int&>(rhs.fd_) = -1;
const_cast<MlockConfig&>(rhs.mlock_config_) = MlockConfig::NoMlock;
}

~MmapDataLoader() override;

__ET_NODISCARD Result<FreeableBuffer> load(
size_t offset,
size_t size,
const DataLoader::SegmentInfo& segment_info) override;
const DataLoader::SegmentInfo& segment_info) const override;

__ET_NODISCARD Result<size_t> size() const override;

Expand All @@ -111,11 +111,11 @@ class MmapDataLoader : public DataLoader {
MmapDataLoader& operator=(const MmapDataLoader&) = delete;
MmapDataLoader& operator=(MmapDataLoader&&) = delete;

const char* file_name_; // String data is owned by the instance.
size_t file_size_;
size_t page_size_;
int fd_; // Owned by the instance.
MlockConfig mlock_config_;
const char* const file_name_; // String data is owned by the instance.
const size_t file_size_;
const size_t page_size_;
const int fd_; // Owned by the instance.
const MlockConfig mlock_config_;
};

} // namespace util
Expand Down
4 changes: 2 additions & 2 deletions extension/data_loader/shared_ptr_data_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ namespace util {
*
* This can be used to wrap data that was allocated elsewhere.
*/
class SharedPtrDataLoader : public DataLoader {
class SharedPtrDataLoader final : public DataLoader {
public:
SharedPtrDataLoader(std::shared_ptr<void> data, size_t size)
: data_(data), size_(size) {}

__ET_NODISCARD Result<FreeableBuffer> load(
size_t offset,
size_t size,
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) override {
__ET_UNUSED const DataLoader::SegmentInfo& segment_info) const override {
ET_CHECK_OR_RETURN_ERROR(
offset + size <= size_,
InvalidArgument,
Expand Down
4 changes: 2 additions & 2 deletions runtime/core/data_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class DataLoader {
* @returns a `FreeableBuffer` that owns the loaded data.
*/
__ET_NODISCARD virtual Result<FreeableBuffer>
load(size_t offset, size_t size, const SegmentInfo& segment_info) = 0;
load(size_t offset, size_t size, const SegmentInfo& segment_info) const = 0;

/**
* Loads data from the underlying data source into the provided buffer.
Expand All @@ -108,7 +108,7 @@ class DataLoader {
size_t offset,
size_t size,
const SegmentInfo& segment_info,
void* buffer) {
void* buffer) const {
// Using a stub implementation here instead of pure virtual to expand the
// data_loader interface in a backwards compatible way.
(void)buffer;
Expand Down
10 changes: 6 additions & 4 deletions runtime/executor/test/backend_integration_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ StubBackend StubBackend::singleton_;
* A DataLoader that wraps a real DataLoader and records the operations
* performed on it and the FreeableBuffers it loads.
*/
class DataLoaderSpy : public DataLoader {
class DataLoaderSpy final : public DataLoader {
public:
/// A record of an operation performed on this DataLoader.
struct Operation {
Expand All @@ -178,8 +178,10 @@ class DataLoaderSpy : public DataLoader {

explicit DataLoaderSpy(DataLoader* delegate) : delegate_(delegate) {}

Result<FreeableBuffer>
load(size_t offset, size_t size, const SegmentInfo& segment_info) override {
Result<FreeableBuffer> load(
size_t offset,
size_t size,
const SegmentInfo& segment_info) const override {
Result<FreeableBuffer> buf = delegate_->load(offset, size, segment_info);
if (!buf.ok()) {
return buf.error();
Expand Down Expand Up @@ -268,7 +270,7 @@ class DataLoaderSpy : public DataLoader {
/// The real loader to delegate to.
DataLoader* delegate_;

std::vector<Operation> operations_;
mutable std::vector<Operation> operations_;
};

constexpr size_t kDefaultNonConstMemBytes = 32 * 1024;
Expand Down
Loading