Skip to content

Commit

Permalink
Enable performance-* checks and fix findings
Browse files Browse the repository at this point in the history
  • Loading branch information
Neverlord committed Sep 13, 2022
1 parent 1ddc6f7 commit 7a2ef78
Show file tree
Hide file tree
Showing 32 changed files with 136 additions and 127 deletions.
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

0 comments on commit 7a2ef78

Please sign in to comment.