Skip to content
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

chore: get rid of MutableSlice #3952

Merged
merged 2 commits into from
Oct 23, 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
13 changes: 0 additions & 13 deletions src/core/core_types.h

This file was deleted.

8 changes: 4 additions & 4 deletions src/core/interpreter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ optional<int> FetchKey(lua_State* lua, const char* key) {
return type;
}

void SetGlobalArrayInternal(lua_State* lua, const char* name, MutSliceSpan args) {
void SetGlobalArrayInternal(lua_State* lua, const char* name, Interpreter::SliceSpan args) {
lua_createtable(lua, args.size(), 0);
for (size_t j = 0; j < args.size(); j++) {
lua_pushlstring(lua, args[j].data(), args[j].size());
Expand Down Expand Up @@ -652,7 +652,7 @@ auto Interpreter::RunFunction(string_view sha, std::string* error) -> RunResult
return err == 0 ? RUN_OK : RUN_ERR;
}

void Interpreter::SetGlobalArray(const char* name, MutSliceSpan args) {
void Interpreter::SetGlobalArray(const char* name, SliceSpan args) {
SetGlobalArrayInternal(lua_, name, args);
}

Expand Down Expand Up @@ -952,7 +952,7 @@ int Interpreter::RedisGenericCommand(bool raise_error, bool async, ObjectExplore
}

char name_buffer[32]; // backing storage for cmd name
absl::FixedArray<absl::Span<char>, 4> args(argc);
absl::FixedArray<string_view, 4> args(argc);

// Copy command name to name_buffer and set it as first arg.
unsigned name_len = lua_rawlen(lua_, 1);
Expand Down Expand Up @@ -1004,7 +1004,7 @@ int Interpreter::RedisGenericCommand(bool raise_error, bool async, ObjectExplore
explorer = &*translator;
}

redis_func_(CallArgs{MutSliceSpan{args}, &buffer_, explorer, async, raise_error, &raise_error});
redis_func_(CallArgs{SliceSpan{args}, &buffer_, explorer, async, raise_error, &raise_error});
cmd_depth_--;

// Shrink reusable buffer if it's too big.
Expand Down
9 changes: 6 additions & 3 deletions src/core/interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@

#pragma once

#include <absl/types/span.h>

#include <functional>
#include <optional>
#include <string_view>

#include "core/core_types.h"
#include "util/fibers/synchronization.h"

typedef struct lua_State lua_State;
Expand Down Expand Up @@ -40,10 +41,12 @@ class ObjectExplorer {

class Interpreter {
public:
using SliceSpan = absl::Span<const std::string_view>;

// Arguments received from redis.call
struct CallArgs {
// Full arguments, including cmd name.
MutSliceSpan args;
SliceSpan args;

// Pointer to backing storage for args (excluding cmd name).
// Moving can invalidate arg slice pointers. Moved by async to re-use buffer.
Expand Down Expand Up @@ -93,7 +96,7 @@ class Interpreter {
RUN_ERR = 2,
};

void SetGlobalArray(const char* name, MutSliceSpan args);
void SetGlobalArray(const char* name, SliceSpan args);

// Runs already added function sha returned by a successful call to AddFunction().
// Returns: true if the call succeeded, otherwise fills error and returns false.
Expand Down
9 changes: 5 additions & 4 deletions src/core/interpreter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class TestSerializer : public ObjectExplorer {
}
};

using SliceSpan = Interpreter::SliceSpan;
class InterpreterTest : public ::testing::Test {
protected:
InterpreterTest() {
Expand All @@ -99,12 +100,12 @@ class InterpreterTest : public ::testing::Test {
};

void InterpreterTest::SetGlobalArray(const char* name, const vector<string_view>& vec) {
vector<MutableSlice> slices(vec.size());
vector<string_view> slices(vec.size());
for (size_t i = 0; i < vec.size(); ++i) {
strings_.emplace_back(new string(vec[i]));
slices[i] = MutableSlice{*strings_.back()};
slices[i] = string_view{*strings_.back()};
}
intptr_.SetGlobalArray(name, MutSliceSpan{slices});
intptr_.SetGlobalArray(name, SliceSpan{slices});
}

bool InterpreterTest::Execute(string_view script) {
Expand Down Expand Up @@ -329,7 +330,7 @@ TEST_F(InterpreterTest, CallArray) {

TEST_F(InterpreterTest, ArgKeys) {
vector<string> vec_arr{};
vector<MutableSlice> slices;
vector<string_view> slices;
SetGlobalArray("ARGV", {"foo", "bar"});
SetGlobalArray("KEYS", {"key1", "key2"});
EXPECT_TRUE(Execute("return {ARGV[1], KEYS[1], KEYS[2]}"));
Expand Down
1 change: 0 additions & 1 deletion src/core/search/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <vector>

#include "base/pmr/memory_resource.h"
#include "core/core_types.h"
#include "core/string_map.h"

namespace dfly::search {
Expand Down
2 changes: 0 additions & 2 deletions src/core/small_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#include <cstdint>
#include <string_view>

#include "core/core_types.h"

namespace dfly {

// blob strings of upto ~256B. Small sizes are probably predominant
Expand Down
19 changes: 0 additions & 19 deletions src/core/uring.h

This file was deleted.

7 changes: 5 additions & 2 deletions src/facade/dragonfly_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "base/io_buf.h"
#include "base/logging.h"
#include "core/heap_size.h"
#include "core/uring.h"
#include "facade/conn_context.h"
#include "facade/dragonfly_listener.h"
#include "facade/memcache_parser.h"
Expand All @@ -30,6 +29,10 @@
#include "util/tls/tls_socket.h"
#endif

#ifdef __linux__
#include "util/fibers/uring_file.h"
#endif

using namespace std;
using facade::operator""_MB;

Expand Down Expand Up @@ -1341,7 +1344,7 @@ bool Connection::ShouldEndDispatchFiber(const MessageHandle& msg) {
void Connection::SquashPipeline() {
DCHECK_EQ(dispatch_q_.size(), pending_pipeline_cmd_cnt_);

vector<CmdArgList> squash_cmds;
vector<ArgSlice> squash_cmds;
squash_cmds.reserve(dispatch_q_.size());

for (auto& msg : dispatch_q_) {
Expand Down
2 changes: 1 addition & 1 deletion src/facade/dragonfly_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Connection : public util::Connection {
// The capacity is chosen so that we allocate a fully utilized (256 bytes) block.
using StorageType = absl::InlinedVector<char, kReqStorageSize, mi_stl_allocator<char>>;

absl::InlinedVector<MutableSlice, 6> args;
absl::InlinedVector<std::string_view, 6> args;
StorageType storage;
};

Expand Down
25 changes: 6 additions & 19 deletions src/facade/facade_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,12 @@ constexpr size_t kSanitizerOverhead = 0u;

enum class Protocol : uint8_t { MEMCACHE = 1, REDIS = 2 };

using MutableSlice = absl::Span<char>;
using CmdArgList = absl::Span<MutableSlice>;
using CmdArgVec = std::vector<MutableSlice>;
using MutableSlice = std::string_view;
using CmdArgList = absl::Span<const std::string_view>;
using CmdArgVec = std::vector<std::string_view>;
using ArgSlice = absl::Span<const std::string_view>;
using OwnedArgSlice = absl::Span<const std::string>;

inline std::string_view ToSV(MutableSlice slice) {
return std::string_view{slice.data(), slice.size()};
}

inline std::string_view ToSV(std::string_view slice) {
return slice;
}
Expand All @@ -57,13 +53,8 @@ inline std::string_view ToSV(std::string&& slice) = delete;

constexpr auto kToSV = [](auto&& v) { return ToSV(std::forward<decltype(v)>(v)); };

inline std::string_view ArgS(CmdArgList args, size_t i) {
auto arg = args[i];
return {arg.data(), arg.size()};
}

inline auto ArgS(CmdArgList args) {
return base::it::Transform(kToSV, base::it::Range{args.begin(), args.end()});
inline std::string_view ArgS(ArgSlice args, size_t i) {
return args[i];
}

struct ArgRange {
Expand Down Expand Up @@ -95,7 +86,7 @@ struct ArgRange {
return std::visit([idx](const auto& span) { return facade::ToSV(span[idx]); }, span);
}

std::variant<CmdArgList, ArgSlice, OwnedArgSlice> span;
std::variant<ArgSlice, OwnedArgSlice> span;
};
struct ConnectionStats {
size_t read_buf_capacity = 0; // total capacity of input buffers
Expand Down Expand Up @@ -179,10 +170,6 @@ struct ErrorReply {
std::optional<OpStatus> status{std::nullopt};
};

inline MutableSlice ToMSS(absl::Span<uint8_t> span) {
return MutableSlice{reinterpret_cast<char*>(span.data()), span.size()};
}

constexpr inline unsigned long long operator""_MB(unsigned long long x) {
return 1024L * 1024L * x;
}
Expand Down
4 changes: 2 additions & 2 deletions src/facade/ok_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ namespace {

class OkService : public ServiceInterface {
public:
void DispatchCommand(CmdArgList args, ConnectionContext* cntx) final {
void DispatchCommand(ArgSlice args, ConnectionContext* cntx) final {
cntx->SendOk();
}

size_t DispatchManyCommands(absl::Span<CmdArgList> args_lists, ConnectionContext* cntx) final {
size_t DispatchManyCommands(absl::Span<ArgSlice> args_lists, ConnectionContext* cntx) final {
for (auto args : args_lists)
DispatchCommand(args, cntx);
return args_lists.size();
Expand Down
2 changes: 1 addition & 1 deletion src/facade/resp_expr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ void RespExpr::VecToArgList(const Vec& src, CmdArgVec* dest) {
for (size_t i = 0; i < src.size(); ++i) {
DCHECK(src[i].type == RespExpr::STRING);

(*dest)[i] = ToMSS(src[i].GetBuf());
(*dest)[i] = ToSV(src[i].GetBuf());
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/facade/service_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ class ServiceInterface {
virtual ~ServiceInterface() {
}

virtual void DispatchCommand(CmdArgList args, ConnectionContext* cntx) = 0;
virtual void DispatchCommand(ArgSlice args, ConnectionContext* cntx) = 0;

// Returns number of processed commands
virtual size_t DispatchManyCommands(absl::Span<CmdArgList> args_list,
ConnectionContext* cntx) = 0;
virtual size_t DispatchManyCommands(absl::Span<ArgSlice> args_list, ConnectionContext* cntx) = 0;

virtual void DispatchMC(const MemcacheParser::Command& cmd, std::string_view value,
ConnectionContext* cntx) = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/server/acl/validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ extern "C" {
namespace dfly::acl {

[[nodiscard]] bool IsUserAllowedToInvokeCommand(const ConnectionContext& cntx, const CommandId& id,
CmdArgList tail_args) {
ArgSlice tail_args) {
if (cntx.skip_acl_validation) {
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/server/command_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ CommandRegistry::FamiliesVec CommandRegistry::GetFamilies() {
return std::move(family_of_commands_);
}

std::pair<const CommandId*, CmdArgList> CommandRegistry::FindExtended(string_view cmd,
CmdArgList tail_args) const {
std::pair<const CommandId*, ArgSlice> CommandRegistry::FindExtended(string_view cmd,
ArgSlice tail_args) const {
if (cmd == RenamedOrOriginal("ACL"sv)) {
if (tail_args.empty()) {
return {Find(cmd), {}};
Expand Down
4 changes: 2 additions & 2 deletions src/server/command_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ class CommandRegistry {
using FamiliesVec = std::vector<std::vector<std::string>>;
FamiliesVec GetFamilies();

std::pair<const CommandId*, facade::CmdArgList> FindExtended(std::string_view cmd,
facade::CmdArgList tail_args) const;
std::pair<const CommandId*, facade::ArgSlice> FindExtended(std::string_view cmd,
facade::ArgSlice tail_args) const;

private:
absl::flat_hash_map<std::string, CommandId> cmd_map_;
Expand Down
8 changes: 4 additions & 4 deletions src/server/conn_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace dfly {
using namespace std;
using namespace facade;

StoredCmd::StoredCmd(const CommandId* cid, CmdArgList args, facade::ReplyMode mode)
StoredCmd::StoredCmd(const CommandId* cid, ArgSlice args, facade::ReplyMode mode)
: cid_{cid}, buffer_{}, sizes_(args.size()), reply_mode_{mode} {
size_t total_size = 0;
for (auto args : args) {
Expand All @@ -38,7 +38,7 @@ StoredCmd::StoredCmd(const CommandId* cid, CmdArgList args, facade::ReplyMode mo
}
}

StoredCmd::StoredCmd(string&& buffer, const CommandId* cid, CmdArgList args, facade::ReplyMode mode)
StoredCmd::StoredCmd(string&& buffer, const CommandId* cid, ArgSlice args, facade::ReplyMode mode)
: cid_{cid}, buffer_{std::move(buffer)}, sizes_(args.size()), reply_mode_{mode} {
for (unsigned i = 0; i < args.size(); i++) {
// Assume tightly packed list.
Expand All @@ -47,7 +47,7 @@ StoredCmd::StoredCmd(string&& buffer, const CommandId* cid, CmdArgList args, fac
}
}

void StoredCmd::Fill(CmdArgList args) {
void StoredCmd::Fill(absl::Span<std::string_view> args) {
DCHECK_GE(args.size(), sizes_.size());

unsigned offset = 0;
Expand Down Expand Up @@ -162,7 +162,7 @@ vector<unsigned> ChangeSubscriptions(bool pattern, CmdArgList args, bool to_add,

// Gather all the channels we need to subscribe to / remove.
size_t i = 0;
for (string_view channel : ArgS(args)) {
for (string_view channel : args) {
if (to_add && local_store.emplace(channel).second)
csu.Record(channel);
else if (!to_add && local_store.erase(channel) > 0)
Expand Down
7 changes: 3 additions & 4 deletions src/server/conn_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,10 @@ struct FlowInfo;
// Used for storing MULTI/EXEC commands.
class StoredCmd {
public:
StoredCmd(const CommandId* cid, CmdArgList args,
facade::ReplyMode mode = facade::ReplyMode::FULL);
StoredCmd(const CommandId* cid, ArgSlice args, facade::ReplyMode mode = facade::ReplyMode::FULL);

// Create on top of already filled tightly-packed buffer.
StoredCmd(std::string&& buffer, const CommandId* cid, CmdArgList args,
StoredCmd(std::string&& buffer, const CommandId* cid, ArgSlice args,
facade::ReplyMode mode = facade::ReplyMode::FULL);

size_t NumArgs() const;
Expand All @@ -40,7 +39,7 @@ class StoredCmd {

// Fill the arg list with stored arguments, it should be at least of size NumArgs().
// Between filling and invocation, cmd should NOT be moved.
void Fill(CmdArgList args);
void Fill(absl::Span<std::string_view> args);

void Fill(CmdArgVec* dest) {
dest->resize(sizes_.size());
Expand Down
4 changes: 2 additions & 2 deletions src/server/debugcmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void DoPopulateBatch(string_view type, string_view prefix, size_t val_size, bool
local_tx->StartMultiNonAtomic();
boost::intrusive_ptr<Transaction> stub_tx =
new Transaction{local_tx.get(), EngineShard::tlocal()->shard_id(), nullopt};
absl::InlinedVector<MutableSlice, 5> args_view;
absl::InlinedVector<string_view, 5> args_view;
facade::CapturingReplyBuilder crb;
ConnectionContext local_cntx{cntx, stub_tx.get(), &crb};

Expand All @@ -162,7 +162,7 @@ void DoPopulateBatch(string_view type, string_view prefix, size_t val_size, bool

args_view.clear();
for (auto& arg : args) {
args_view.push_back(absl::MakeSpan(arg));
args_view.push_back(arg);
}
auto args_span = absl::MakeSpan(args_view);

Expand Down
Loading
Loading