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

Enable performance-* checks and fix findings #279

Merged
merged 1 commit into from
Sep 20, 2022
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
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Checks: -*,
bugprone-*,
clang-analyzer-*,
performance-*,
-bugprone-easily-swappable-parameters,
-clang-analyzer-security.insecureAPI.rand,
-clang-analyzer-webkit*,
Expand Down
2 changes: 1 addition & 1 deletion include/broker/alm/routing_table.hh
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void erase(routing_table& tbl, const endpoint_id& whom,
impl(whom);
while (!unreachable_peers.empty()) {
// Our lambda modifies unreachable_peers, so we can't use iterators here.
endpoint_id peer = std::move(unreachable_peers.back());
endpoint_id peer = unreachable_peers.back();
unreachable_peers.pop_back();
impl(peer);
on_remove(peer);
Expand Down
22 changes: 11 additions & 11 deletions include/broker/configuration.hh
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public:

configuration();

configuration(configuration&&);
configuration(configuration&&) noexcept;

/// Constructs a configuration with non-default Broker options.
explicit configuration(broker_options opts);
Expand Down Expand Up @@ -177,20 +177,20 @@ public:
std::string_view description);

template <class T>
std::enable_if_t<std::is_integral_v<T>> set(std::string key, T val) {
std::enable_if_t<std::is_integral_v<T>> set(std::string_view key, T val) {
if constexpr (std::is_same_v<T, bool>)
set_bool(std::move(key), val);
set_bool(key, val);
else if constexpr (std::is_signed_v<T>)
set_i64(std::move(key), val);
set_i64(key, val);
else
set_u64(std::move(key), val);
set_u64(key, val);
}

void set(std::string key, timespan val);
void set(std::string_view key, timespan val);

void set(std::string key, std::string val);
void set(std::string_view key, std::string val);

void set(std::string key, std::vector<std::string> val);
void set(std::string_view key, std::vector<std::string> val);

std::optional<int64_t> read_i64(std::string_view key, int64_t min_val,
int64_t max_val) const;
Expand Down Expand Up @@ -223,11 +223,11 @@ public:
void init(int argc, char** argv);

private:
void set_i64(std::string key, int64_t val);
void set_i64(std::string_view key, int64_t val);

void set_u64(std::string key, uint64_t val);
void set_u64(std::string_view key, uint64_t val);

void set_bool(std::string key, bool val);
void set_bool(std::string_view key, bool val);

std::unique_ptr<impl> impl_;
};
Expand Down
7 changes: 4 additions & 3 deletions include/broker/endpoint.hh
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ public:
/// this function may block.
/// @returns `true` if `what` was added before the timeout, `false` otherwise.
[[nodiscard]] bool
await_filter_entry(topic what,
await_filter_entry(const topic& what,
timespan timeout = defaults::await_peer_timeout);

// -- worker management ------------------------------------------------------
Expand Down Expand Up @@ -514,9 +514,10 @@ protected:
worker subscriber_;

private:
worker do_subscribe(filter_type&& topics, detail::sink_driver_ptr driver);
worker do_subscribe(filter_type&& topics,
const detail::sink_driver_ptr& driver);

worker do_publish_all(detail::source_driver_ptr driver);
worker do_publish_all(const detail::source_driver_ptr& driver);

template <class F>
worker make_worker(F fn);
Expand Down
3 changes: 1 addition & 2 deletions include/broker/error.hh
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,7 @@ public:

template <ec Code>
static error make(ec_constant<Code>, endpoint_id node, std::string msg) {
return make_impl(Code, endpoint_info{std::move(node), std::nullopt},
std::move(msg));
return make_impl(Code, endpoint_info{node, std::nullopt}, std::move(msg));
}

private:
Expand Down
2 changes: 1 addition & 1 deletion include/broker/internal/clone_actor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public:

error consume_nil(consumer_type* src);

void close(consumer_type* src, error);
void close(consumer_type* src, const error&);

void send(consumer_type*, channel_type::cumulative_ack);

Expand Down
2 changes: 1 addition & 1 deletion include/broker/internal/core_actor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public:
/// `init_new_peer` with the buffers that connect to the worker.
caf::error init_new_peer(endpoint_id peer, const network_info& addr,
const filter_type& filter,
pending_connection_ptr conn);
const pending_connection_ptr& conn);

/// Connects the input and output buffers for a new client to our central
/// merge point.
Expand Down
2 changes: 1 addition & 1 deletion include/broker/internal/json_client.hh
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public:

static std::string_view default_serialization_failed_error();

void init(filter_type filter, out_t out,
void init(const filter_type& filter, const out_t& out,
caf::async::consumer_resource<data_message> core_pull);
};

Expand Down
2 changes: 1 addition & 1 deletion include/broker/internal/master_actor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public:

error consume_nil(consumer_type* src);

void close(consumer_type* src, error);
void close(consumer_type* src, const error&);

void send(consumer_type*, channel_type::cumulative_ack);

Expand Down
4 changes: 2 additions & 2 deletions include/broker/internal/store_actor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public:
[this](atom::increment, detail::shared_store_state_ptr ptr) {
attached_states.emplace(std::move(ptr), size_t{0}).first->second += 1;
},
[this](atom::decrement, detail::shared_store_state_ptr ptr) {
[this](atom::decrement, const detail::shared_store_state_ptr& ptr) {
auto& xs = attached_states;
if (auto i = xs.find(ptr); i != xs.end())
if (--(i->second) == 0)
Expand Down Expand Up @@ -169,7 +169,7 @@ public:
// -- convenience functions --------------------------------------------------

/// Sends a delayed message by using the endpoint's clock.
void send_later(caf::actor hdl, timespan delay, caf::message msg);
void send_later(const caf::actor& hdl, timespan delay, caf::message msg);

// -- member variables -------------------------------------------------------

Expand Down
5 changes: 3 additions & 2 deletions include/broker/internal/web_socket.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ using connect_event_t = std::pair<pull_t, push_t>;
using on_connect_t =
std::function<void(const caf::settings&, connect_event_t&)>;

expected<uint16_t> launch(caf::actor_system& sys, openssl_options_ptr ssl_cfg,
std::string addr, uint16_t port, bool reuse_addr,
expected<uint16_t> launch(caf::actor_system& sys,
const openssl_options_ptr& ssl_cfg, std::string addr,
uint16_t port, bool reuse_addr,
const std::string& allowed_path,
on_connect_t on_connect);

Expand Down
2 changes: 1 addition & 1 deletion include/broker/message.hh
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ using packed_message =
inline packed_message make_packed_message(packed_message_type type,
uint16_t ttl, topic dst,
std::vector<std::byte> bytes) {
return packed_message{type, ttl, dst, std::move(bytes)};
return packed_message{type, ttl, std::move(dst), std::move(bytes)};
}

/// @relates packed_message
Expand Down
4 changes: 2 additions & 2 deletions include/broker/status.hh
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ public:
template <sc S>
static status make(endpoint_id node, std::string msg) {
static_assert(sc_has_endpoint_info_v<S>);
return {S, endpoint_info{std::move(node), std::nullopt}, std::move(msg)};
return {S, endpoint_info{node, std::nullopt}, std::move(msg)};
}

template <sc S>
static status make(sc_constant<S>, endpoint_id node, std::string msg) {
static_assert(sc_has_endpoint_info_v<S>);
return {S, endpoint_info{std::move(node), std::nullopt}, std::move(msg)};
return {S, endpoint_info{node, std::nullopt}, std::move(msg)};
}

/// Default-constructs an unspecified status.
Expand Down
23 changes: 11 additions & 12 deletions include/broker/store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ public:

store() = default;

store(store&&) = default;
store(store&&) noexcept = default;

store(const store&);

store& operator=(store&&);
store& operator=(store&&) noexcept;

store& operator=(const store&);

Expand Down Expand Up @@ -204,7 +204,7 @@ public:
break;
}

add(std::move(key), std::move(amount), init_type, std::move(expiry));
add(std::move(key), std::move(amount), init_type, expiry);
}

/// Decrements a value by a given amount. This is supported for all
Expand All @@ -213,23 +213,23 @@ public:
/// @param value The amount to decrement the value.
/// @param expiry An optional new expiration time for *key*.
void decrement(data key, data amount, std::optional<timespan> expiry = {}) {
subtract(std::move(key), std::move(amount), std::move(expiry));
subtract(std::move(key), std::move(amount), expiry);
}

/// Appends a string to another one.
/// @param key The key of the string to which to append.
/// @param str The string to append.
/// @param expiry An optional new expiration time for *key*.
void append(data key, data str, std::optional<timespan> expiry = {}) {
add(std::move(key), std::move(str), data::type::string, std::move(expiry));
add(std::move(key), std::move(str), data::type::string, expiry);
}

/// Inserts an index into a set.
/// @param key The key of the set into which to insert the value.
/// @param index The index to insert.
/// @param expiry An optional new expiration time for *key*.
void insert_into(data key, data index, std::optional<timespan> expiry = {}) {
add(std::move(key), std::move(index), data::type::set, std::move(expiry));
add(std::move(key), std::move(index), data::type::set, expiry);
}

/// Inserts an index into a table.
Expand All @@ -241,31 +241,30 @@ public:
void insert_into(data key, data index, data value,
std::optional<timespan> expiry = {}) {
add(std::move(key), vector({std::move(index), std::move(value)}),
data::type::table, std::move(expiry));
data::type::table, expiry);
}

/// Removes am index from a set or table.
/// @param key The key of the set/table from which to remove the value.
/// @param index The index to remove.
/// @param expiry An optional new expiration time for *key*.
void remove_from(data key, data index, std::optional<timespan> expiry = {}) {
subtract(std::move(key), std::move(index), std::move(expiry));
subtract(std::move(key), std::move(index), expiry);
}

/// Appends a value to a vector.
/// @param key The key of the vector to which to append the value.
/// @param value The value to append.
/// @param expiry An optional new expiration time for *key*.
void push(data key, data value, std::optional<timespan> expiry = {}) {
add(std::move(key), std::move(value), data::type::vector,
std::move(expiry));
add(std::move(key), std::move(value), data::type::vector, expiry);
}

/// Removes the last value of a vector.
/// @param key The key of the vector from which to remove the last value.
/// @param expiry An optional new expiration time for *key*.
void pop(data key, std::optional<timespan> expiry = {}) {
subtract(key, key, std::move(expiry));
void pop(const data& key, std::optional<timespan> expiry = {}) {
subtract(key, key, expiry);
}

// --await-idle-start
Expand Down
2 changes: 1 addition & 1 deletion include/broker/store_event.hh
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public:

entity_id publisher() const {
if (auto value = to<endpoint_id>((*xs_)[5])) {
return {std::move(*value), get<uint64_t>((*xs_)[6])};
return {*value, get<uint64_t>((*xs_)[6])};
}
return {};
}
Expand Down
2 changes: 1 addition & 1 deletion include/broker/subnet.hh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public:
subnet();

/// Construct subnet from an address and length.
subnet(address addr, uint8_t length);
subnet(const address& addr, uint8_t length);

/// @return whether an address is contained within this subnet.
bool contains(const address& addr) const;
Expand Down
4 changes: 2 additions & 2 deletions src/broker-node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ void relay_mode(broker::endpoint& ep, topic_list topics) {
}
return true;
};
auto in = ep.make_subscriber(topics);
auto in = ep.make_subscriber(std::move(topics));
auto& cfg = broker::internal::endpoint_access{&ep}.cfg();
if (get_or(cfg, "verbose", false) && get_or(cfg, "rate", false)) {
auto timeout = std::chrono::system_clock::now();
Expand Down Expand Up @@ -368,7 +368,7 @@ void ping_mode(broker::endpoint& ep, topic_list topics) {
void pong_mode(broker::endpoint& ep, topic_list topics) {
assert(topics.size() > 0);
verbose::println("receive pings from topics ", topics);
auto in = ep.make_subscriber(topics);
auto in = ep.make_subscriber(std::move(topics));
for (;;) {
auto x = in.get();
auto& val = get_data(x);
Expand Down
2 changes: 1 addition & 1 deletion src/broker-pipe.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ void subscribe_mode_stream(broker::endpoint& ep, const std::string& topic_str,
// Init.
[](size_t& msgs) { msgs = 0; },
// OnNext.
[cap](size_t& msgs, data_message x) {
[cap](size_t& msgs, const data_message& x) {
++msg_count;
if (!rate)
print_line(std::cout, to_string(x));
Expand Down
26 changes: 13 additions & 13 deletions src/configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ configuration::configuration() : configuration(skip_init) {
init(0, nullptr);
}

configuration::configuration(configuration&& other)
configuration::configuration(configuration&& other) noexcept
: impl_(std::move(other.impl_)) {
// cannot '= default' this because impl is incomplete in the header.
}
Expand Down Expand Up @@ -443,28 +443,28 @@ void configuration::add_option(std::vector<std::string>* dst,
description);
}

void configuration::set(std::string key, timespan val) {
impl_->set(std::move(key), val);
void configuration::set(std::string_view key, timespan val) {
impl_->set(key, val);
}

void configuration::set(std::string key, std::string val) {
impl_->set(std::move(key), std::move(val));
void configuration::set(std::string_view key, std::string val) {
impl_->set(key, std::move(val));
}

void configuration::set(std::string key, std::vector<std::string> val) {
impl_->set(std::move(key), std::move(val));
void configuration::set(std::string_view key, std::vector<std::string> val) {
impl_->set(key, std::move(val));
}

void configuration::set_i64(std::string key, int64_t val) {
impl_->set(std::move(key), val);
void configuration::set_i64(std::string_view key, int64_t val) {
impl_->set(key, val);
}

void configuration::set_u64(std::string key, uint64_t val) {
impl_->set(std::move(key), val);
void configuration::set_u64(std::string_view key, uint64_t val) {
impl_->set(key, val);
}

void configuration::set_bool(std::string key, bool val) {
impl_->set(std::move(key), val);
void configuration::set_bool(std::string_view key, bool val) {
impl_->set(key, val);
}

std::optional<int64_t> configuration::read_i64(std::string_view key,
Expand Down
Loading