mirror
/
transmission
mirror of https://github.com/transmission/transmission
1
0
Fork 0
Code Issues Releases Wiki Activity

refactor: thread-safe `lru-cache` and `tr_open_files` (#6230)

This commit is contained in:
Yat Ho 2023-11-12 11:09:23 +08:00 committed by GitHub
parent a575be778f
commit 40fe56f33e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 134 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
libtransmission/inout.cc
View File

@ -34,7 +34,7 @@ namespace
bool readEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t* buf, uint64_t buflen, tr_error* error)
{
while (buflen > 0)
while (buflen > 0U)
{
auto n_read = uint64_t{};
@ -53,7 +53,7 @@ bool readEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t* buf, uint64_
bool writeEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t const* buf, uint64_t buflen, tr_error* error)
{
while (buflen > 0)
while (buflen > 0U)
{
auto n_written = uint64_t{};
@ -112,10 +112,10 @@ void readOrWriteBytes(
bool const do_write = io_mode == IoMode::Write;
auto const file_size = tor->file_size(file_index);
TR_ASSERT(file_size == 0 || file_offset < file_size);
TR_ASSERT(file_size == 0U || file_offset < file_size);
TR_ASSERT(file_offset + buflen <= file_size);
if (file_size == 0)
if (file_size == 0U)
{
return;
}
@ -128,9 +128,9 @@ void readOrWriteBytes(
// --- Find the fd
auto fd = session->openFiles().get(tor->id(), file_index, do_write);
auto fd_top = session->openFiles().get(tor->id(), file_index, do_write);
auto filename = tr_pathbuf{};
if (!fd && !getFilename(filename, tor, file_index, io_mode))
if (!fd_top && !getFilename(filename, tor, file_index, io_mode))
{
auto const err = ENOENT;
error->set(
@ -143,20 +143,20 @@ void readOrWriteBytes(
return;
}
if (!fd) // not in the cache, so open or create it now
if (!fd_top) // not in the cache, so open or create it now
{
// open (and maybe create) the file
auto const prealloc = (!do_write || !tor->file_is_wanted(file_index)) ? TR_PREALLOCATE_NONE :
tor->session->preallocationMode();
fd = session->openFiles().get(tor->id(), file_index, do_write, filename, prealloc, file_size);
if (fd && do_write)
fd_top = session->openFiles().get(tor->id(), file_index, do_write, filename, prealloc, file_size);
if (fd_top && do_write)
{
// make a note that we just created a file
tor->session->add_file_created();
}
}
if (!fd) // couldn't create/open it either
if (!fd_top) // couldn't create/open it either
{
auto const errnum = errno;
error->set(
@ -170,10 +170,11 @@ void readOrWriteBytes(
return;
}
auto const& [fd, tag] = *fd_top;
switch (io_mode)
{
case IoMode::Read:
if (!readEntireBuf(*fd, file_offset, buf, buflen, error) && *error)
if (!readEntireBuf(fd, file_offset, buf, buflen, error) && *error)
{
tr_logAddErrorTor(
tor,
@ -187,7 +188,7 @@ void readOrWriteBytes(
break;
case IoMode::Write:
if (!writeEntireBuf(*fd, file_offset, buf, buflen, error) && *error)
if (!writeEntireBuf(fd, file_offset, buf, buflen, error) && *error)
{
tr_logAddErrorTor(
tor,
@ -201,7 +202,7 @@ void readOrWriteBytes(
break;
case IoMode::Prefetch:
tr_sys_file_advise(*fd, file_offset, buflen, TR_SYS_FILE_ADVICE_WILL_NEED);
tr_sys_file_advise(fd, file_offset, buflen, TR_SYS_FILE_ADVICE_WILL_NEED);
break;
}
}
@ -216,7 +217,7 @@ int readOrWritePiece(tr_torrent* tor, IoMode io_mode, tr_block_info::Location lo
auto [file_index, file_offset] = tor->file_offset(loc, false);
while (buflen != 0)
while (buflen != 0U)
{
uint64_t const bytes_this_pass = std::min(uint64_t{ buflen }, uint64_t{ tor->file_size(file_index) - file_offset });
@ -240,7 +241,7 @@ int readOrWritePiece(tr_torrent* tor, IoMode io_mode, tr_block_info::Location lo
buflen -= bytes_this_pass;
++file_index;
file_offset = 0;
file_offset = 0U;
}
return 0;
@ -275,7 +276,7 @@ std::optional<tr_sha1_digest_t> recalculateHash(tr_torrent* tor, tr_piece_index_
{
begin += (begin_byte - block_loc.byte);
}
if (block + 1 == end_block) // `block` may end after `piece` does
if (block + 1U == end_block) // `block` may end after `piece` does
{
end -= (block_loc.byte + block_len - end_byte);
}

89
libtransmission/lru-cache.h
View File

@ -6,44 +6,53 @@
#pragma once
#include <array>
#include <condition_variable>
#include <cstddef> // size_t
#include <cstdint>
#include <functional>
#include <mutex>
#include <optional>
#include <utility>
#include "libtransmission/observable.h"
// A fixed-size cache that erases least-recently-used items to make room for new ones.
template<typename Key, typename Val, std::size_t N>
class tr_lru_cache
{
public:
[[nodiscard]] constexpr Val* get(Key const& key) noexcept
[[nodiscard]] TR_CONSTEXPR23 std::optional<std::pair<Val&, libtransmission::ObserverTag>> get(Key const& key) noexcept
{
if (auto const found = find(key); found != nullptr)
auto const lock = std::lock_guard{ mutex_ };
if (auto* const found = find(key); found != nullptr)
{
found->sequence_ = next_sequence_++;
return &found->val_;
return lock_and_get_entry(*found);
}
return nullptr;
return {};
}
[[nodiscard]] constexpr bool contains(Key const& key) const noexcept
[[nodiscard]] TR_CONSTEXPR23 bool contains(Key const& key) const noexcept
{
return !!find(key);
return find(key) != nullptr;
}
Val& add(Key&& key)
std::pair<Val&, libtransmission::ObserverTag> add(Key&& key)
{
auto& entry = getFreeSlot();
auto const lock = std::lock_guard{ mutex_ };
auto& entry = get_free_slot();
entry.key_ = std::move(key);
entry.sequence_ = next_sequence_++;
key = {};
return entry.val_;
return lock_and_get_entry(entry);
}
void erase(Key const& key)
{
auto const lock = std::lock_guard{ mutex_ };
if (auto* const found = find(key); found != nullptr)
{
this->erase(*found);
@ -52,6 +61,7 @@ public:
void erase_if(std::function<bool(Key const&, Val const&)> test)
{
auto const lock = std::lock_guard{ mutex_ };
for (auto& entry : entries_)
{
if (entry.sequence_ != InvalidSeq && test(entry.key_, entry.val_))
@ -63,44 +73,50 @@ public:
void clear()
{
auto const lock = std::lock_guard{ mutex_ };
for (auto& entry : entries_)
{
erase(entry);
}
}
using PreEraseCallback = std::function<void(Key const&, Val&)>;
void setPreErase(PreEraseCallback&& func)
{
pre_erase_cb_ = std::move(func);
}
private:
PreEraseCallback pre_erase_cb_ = [](Key const&, Val&) {
};
struct Entry
{
Key key_ = {};
Val val_ = {};
size_t in_use_ = 0U;
uint64_t sequence_ = InvalidSeq;
};
std::pair<Val&, libtransmission::ObserverTag> lock_and_get_entry(Entry& entry)
{
auto const lock = std::lock_guard{ mutex_ };
++entry.in_use_;
return std::make_pair(
std::ref(entry.val_),
libtransmission::ObserverTag{ [this, &entry]()
{
auto lock2 = std::unique_lock{ mutex_ };
--entry.in_use_;
TR_ASSERT(entry.in_use_ >= 0U);
lock2.unlock();
cv_.notify_one();
} });
}
void erase(Entry& entry)
{
if (entry.sequence_ != InvalidSeq)
{
pre_erase_cb_(entry.key_, entry.val_);
}
auto const lock = std::lock_guard{ mutex_ };
entry.key_ = {};
entry.val_ = {};
entry.sequence_ = InvalidSeq;
}
[[nodiscard]] constexpr Entry* find(Key const& key) noexcept
[[nodiscard]] TR_CONSTEXPR23 Entry* find(Key const& key) noexcept
{
auto const lock = std::lock_guard{ mutex_ };
for (auto& entry : entries_)
{
if (entry.sequence_ != InvalidSeq && entry.key_ == key)
@ -112,8 +128,9 @@ private:
return nullptr;
}
[[nodiscard]] constexpr Entry const* find(Key const& key) const noexcept
[[nodiscard]] TR_CONSTEXPR23 Entry const* find(Key const& key) const noexcept
{
auto const lock = std::lock_guard{ mutex_ };
for (auto const& entry : entries_)
{
if (entry.sequence_ != InvalidSeq && entry.key_ == key)
@ -125,17 +142,29 @@ private:
return nullptr;
}
Entry& getFreeSlot()
Entry& get_free_slot()
{
auto lock = std::unique_lock{ mutex_ };
cv_.wait(
lock,
[this]() {
return std::any_of(
std::begin(entries_),
std::end(entries_),
[](auto const& entry) { return entry.in_use_ == 0U; });
});
auto const iter = std::min_element(
std::begin(entries_),
std::end(entries_),
[](auto const& a, auto const& b) { return a.sequence_ < b.sequence_; });
this->erase(*iter);
[](auto const& a, auto const& b) { return b.in_use_ > 0U || a.sequence_ < b.sequence_; });
erase(*iter);
return *iter;
}
std::array<Entry, N> entries_;
uint64_t next_sequence_ = 1;
static uint64_t constexpr InvalidSeq = 0;
uint64_t next_sequence_ = 1U;
static uint64_t constexpr InvalidSeq = 0U;
mutable std::recursive_mutex mutex_;
std::condition_variable_any cv_;
};

44
libtransmission/open-files.cc
View File

@ -17,11 +17,14 @@
#include "libtransmission/error.h"
#include "libtransmission/file.h"
#include "libtransmission/log.h"
#include "libtransmission/observable.h"
#include "libtransmission/open-files.h"
#include "libtransmission/tr-assert.h"
#include "libtransmission/tr-strbuf.h"
#include "libtransmission/utils.h" // _()
using namespace std::literals;
namespace
{
@ -32,7 +35,7 @@ namespace
bool preallocate_file_sparse(tr_sys_file_t fd, uint64_t length, tr_error* error)
{
if (length == 0)
if (length == 0U)
{
return true;
}
@ -48,12 +51,12 @@ bool preallocate_file_sparse(tr_sys_file_t fd, uint64_t length, tr_error* error)
if (!TR_ERROR_IS_ENOSPC(local_error.code()))
{
char const zero = '\0';
static char constexpr Zero = '\0';
local_error = {};
/* fallback: the old-style seek-and-write */
if (tr_sys_file_write_at(fd, &zero, 1, length - 1, nullptr, &local_error) &&
if (tr_sys_file_write_at(fd, &Zero, 1, length - 1, nullptr, &local_error) &&
tr_sys_file_truncate(fd, length, &local_error))
{
return true;
@ -72,7 +75,7 @@ bool preallocate_file_sparse(tr_sys_file_t fd, uint64_t length, tr_error* error)
bool preallocate_file_full(tr_sys_file_t fd, uint64_t length, tr_error* error)
{
if (length == 0)
if (length == 0U)
{
return true;
}
@ -122,22 +125,26 @@ bool preallocate_file_full(tr_sys_file_t fd, uint64_t length, tr_error* error)
// ---
std::optional<tr_sys_file_t> tr_open_files::get(tr_torrent_id_t tor_id, tr_file_index_t file_num, bool writable)
std::optional<std::pair<tr_sys_file_t, libtransmission::ObserverTag>> tr_open_files::get(
tr_torrent_id_t tor_id,
tr_file_index_t file_num,
bool writable)
{
if (auto* const found = pool_.get(make_key(tor_id, file_num)); found != nullptr)
if (auto found = pool_.get(make_key(tor_id, file_num)); found)
{
if (writable && !found->writable_)
auto& [entry, tag] = *found;
if (writable && !entry.writable_)
{
return {};
}
return found->fd_;
return std::pair{ entry.fd_, std::move(tag) };
}
return {};
}
std::optional<tr_sys_file_t> tr_open_files::get(
std::optional<std::pair<tr_sys_file_t, libtransmission::ObserverTag>> tr_open_files::get(
tr_torrent_id_t tor_id,
tr_file_index_t file_num,
bool writable,
@ -147,11 +154,12 @@ std::optional<tr_sys_file_t> tr_open_files::get(
{
// is there already an entry
auto key = make_key(tor_id, file_num);
if (auto* const found = pool_.get(key); found != nullptr)
if (auto found = pool_.get(key); found)
{
if (!writable || found->writable_)
auto& [entry, tag] = *found;
if (!writable || entry.writable_)
{
return found->fd_;
return std::pair{ entry.fd_, std::move(tag) };
}
pool_.erase(key); // close so we can re-open as writable
@ -199,20 +207,20 @@ std::optional<tr_sys_file_t> tr_open_files::get(
if (writable && !already_existed && allocation != TR_PREALLOCATE_NONE)
{
bool success = false;
char const* type = nullptr;
auto type = std::string_view{};
if (allocation == TR_PREALLOCATE_FULL)
{
success = preallocate_file_full(fd, file_size, &error);
type = "full";
type = "full"sv;
}
else if (allocation == TR_PREALLOCATE_SPARSE)
{
success = preallocate_file_sparse(fd, file_size, &error);
type = "sparse";
type = "sparse"sv;
}
TR_ASSERT(type != nullptr);
TR_ASSERT(!std::empty(type));
if (!success)
{
@ -245,11 +253,11 @@ std::optional<tr_sys_file_t> tr_open_files::get(
}
// cache it
auto& entry = pool_.add(std::move(key));
auto [entry, tag] = pool_.add(std::move(key));
entry.fd_ = fd;
entry.writable_ = writable;
return fd;
return std::pair{ fd, std::move(tag) };
}
void tr_open_files::close_all()

9
libtransmission/open-files.h
View File

@ -24,9 +24,12 @@
class tr_open_files
{
public:
[[nodiscard]] std::optional<tr_sys_file_t> get(tr_torrent_id_t tor_id, tr_file_index_t file_num, bool writable);
[[nodiscard]] std::optional<std::pair<tr_sys_file_t, libtransmission::ObserverTag>> get(
tr_torrent_id_t tor_id,
tr_file_index_t file_num,
bool writable);
[[nodiscard]] std::optional<tr_sys_file_t> get(
[[nodiscard]] std::optional<std::pair<tr_sys_file_t, libtransmission::ObserverTag>> get(
tr_torrent_id_t tor_id,
tr_file_index_t file_num,
bool writable,
@ -67,6 +70,6 @@ private:
bool writable_ = false;
};
static constexpr size_t MaxOpenFiles = 32;
static constexpr size_t MaxOpenFiles = 32U;
tr_lru_cache<Key, Val, MaxOpenFiles> pool_;
};

51
tests/libtransmission/open-files-test.cc
View File

@ -28,8 +28,8 @@ using OpenFilesTest = libtransmission::test::SessionTest;
TEST_F(OpenFilesTest, getCachedFailsIfNotCached)
{
auto const fd = session_->openFiles().get(0, 0, false);
EXPECT_FALSE(fd);
auto const fd_top = session_->openFiles().get(0, 0, false);
EXPECT_FALSE(fd_top);
}
TEST_F(OpenFilesTest, getOpensIfNotCached)
@ -42,15 +42,16 @@ TEST_F(OpenFilesTest, getOpensIfNotCached)
EXPECT_FALSE(session_->openFiles().get(0, 0, false));
// confirm that we can cache the file
auto fd = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents));
EXPECT_TRUE(fd.has_value());
assert(fd.has_value());
EXPECT_NE(TR_BAD_SYS_FILE, *fd);
auto fd_top = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents));
EXPECT_TRUE(fd_top.has_value());
assert(fd_top.has_value());
auto& [fd, tag] = *fd_top;
EXPECT_NE(TR_BAD_SYS_FILE, fd);
// test the file contents to confirm that fd points to the right file
auto buf = std::array<char, std::size(Contents) + 1>{};
auto bytes_read = uint64_t{};
EXPECT_TRUE(tr_sys_file_read_at(*fd, std::data(buf), std::size(Contents), 0, &bytes_read));
EXPECT_TRUE(tr_sys_file_read_at(fd, std::data(buf), std::size(Contents), 0, &bytes_read));
auto const contents = std::string_view{ std::data(buf), static_cast<size_t>(bytes_read) };
EXPECT_EQ(Contents, contents);
}
@ -73,13 +74,13 @@ TEST_F(OpenFilesTest, getCachedReturnsTheSameFd)
createFileWithContents(filename, Contents);
EXPECT_FALSE(session_->openFiles().get(0, 0, false));
auto const fd1 = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents));
auto const fd2 = session_->openFiles().get(0, 0, false);
EXPECT_TRUE(fd1.has_value());
EXPECT_TRUE(fd2.has_value());
assert(fd1.has_value());
assert(fd2.has_value());
EXPECT_EQ(*fd1, *fd2);
auto const fd1_top = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents));
auto const fd2_top = session_->openFiles().get(0, 0, false);
EXPECT_TRUE(fd1_top.has_value());
EXPECT_TRUE(fd2_top.has_value());
assert(fd1_top.has_value());
assert(fd2_top.has_value());
EXPECT_EQ(fd1_top->first, fd2_top->first);
}
TEST_F(OpenFilesTest, getCachedFailsIfWrongPermissions)
@ -104,13 +105,13 @@ TEST_F(OpenFilesTest, opensInReadOnlyUnlessWritableIsRequested)
createFileWithContents(filename, Contents);
// cache a file read-only mode
auto fd = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents));
EXPECT_TRUE(fd.has_value());
assert(fd.has_value());
auto fd_top = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents));
EXPECT_TRUE(fd_top.has_value());
assert(fd_top.has_value());
// confirm that writing to it fails
auto error = tr_error{};
EXPECT_FALSE(tr_sys_file_write(*fd, std::data(Contents), std::size(Contents), nullptr, &error));
EXPECT_FALSE(tr_sys_file_write(fd_top->first, std::data(Contents), std::size(Contents), nullptr, &error));
EXPECT_TRUE(error);
}
@ -120,14 +121,14 @@ TEST_F(OpenFilesTest, createsMissingFileIfWriteRequested)
auto filename = tr_pathbuf{ sandboxDir(), "/test-file.txt" };
EXPECT_FALSE(tr_sys_path_exists(filename));
auto fd = session_->openFiles().get(0, 0, false);
EXPECT_FALSE(fd);
auto fd_top = session_->openFiles().get(0, 0, false);
EXPECT_FALSE(fd_top);
EXPECT_FALSE(tr_sys_path_exists(filename));
fd = session_->openFiles().get(0, 0, true, filename, TR_PREALLOCATE_FULL, std::size(Contents));
EXPECT_TRUE(fd.has_value());
assert(fd.has_value());
EXPECT_NE(TR_BAD_SYS_FILE, *fd);
fd_top = session_->openFiles().get(0, 0, true, filename, TR_PREALLOCATE_FULL, std::size(Contents));
EXPECT_TRUE(fd_top.has_value());
assert(fd_top.has_value());
EXPECT_NE(TR_BAD_SYS_FILE, fd_top->first);
EXPECT_TRUE(tr_sys_path_exists(filename));
}
@ -196,7 +197,7 @@ TEST_F(OpenFilesTest, closesLeastRecentlyUsedFile)
for (int i = 0; i < LargerThanCacheLimit; ++i)
{
auto filename = tr_pathbuf{ sandboxDir(), fmt::format("/file-{:d}.txt"sv, i) };
results[i] = !!session_->openFiles().get(TorId, i, false);
results[i] = static_cast<bool>(session_->openFiles().get(TorId, i, false));
}
sorted = results;
std::sort(std::begin(sorted), std::end(sorted));