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

refactor(clp-s): Replace instances of std::string const& with std::string_view where it would remove unnecessary conversions to and from std::string. #635

Merged
merged 5 commits into from
Dec 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
8 changes: 4 additions & 4 deletions components/core/src/clp_s/ArchiveWriter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ class ArchiveWriter {
* @return the epoch time corresponding to the string timestamp
*/
epochtime_t ingest_timestamp_entry(
std::string const& key,
std::string_view key,
int32_t node_id,
std::string const& timestamp,
std::string_view timestamp,
uint64_t& pattern_id
) {
return m_timestamp_dict.ingest_entry(key, node_id, timestamp, pattern_id);
Expand All @@ -136,11 +136,11 @@ class ArchiveWriter {
* @param node_id
* @param timestamp
*/
void ingest_timestamp_entry(std::string const& key, int32_t node_id, double timestamp) {
void ingest_timestamp_entry(std::string_view key, int32_t node_id, double timestamp) {
m_timestamp_dict.ingest_entry(key, node_id, timestamp);
}

void ingest_timestamp_entry(std::string const& key, int32_t node_id, int64_t timestamp) {
void ingest_timestamp_entry(std::string_view key, int32_t node_id, int64_t timestamp) {
m_timestamp_dict.ingest_entry(key, node_id, timestamp);
}

Expand Down
10 changes: 10 additions & 0 deletions components/core/src/clp_s/ParsedMessage.hpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#ifndef CLP_S_PARSEDMESSAGE_HPP
#define CLP_S_PARSEDMESSAGE_HPP

#include <cstdint>
#include <map>
gibber9809 marked this conversation as resolved.
Show resolved Hide resolved
#include <string>
#include <string_view>
#include <utility>
#include <variant>

Expand Down Expand Up @@ -34,6 +36,10 @@ class ParsedMessage {
m_message.emplace(node_id, value);
}

inline void add_value(int32_t node_id, std::string_view value) {
m_message.emplace(node_id, std::string{value});
}

/**
* Adds a timestamp value and its encoding to the message for a given MST node ID.
* @param node_id
Expand All @@ -55,6 +61,10 @@ class ParsedMessage {
m_unordered_message.emplace_back(value);
}

inline void add_unordered_value(std::string_view value) {
m_unordered_message.emplace_back(std::string{value});
}

/**
* Clears the message
*/
Expand Down
10 changes: 6 additions & 4 deletions components/core/src/clp_s/TimestampDictionaryWriter.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "TimestampDictionaryWriter.hpp"

#include <cstdint>
#include <sstream>
#include <string_view>

#include "Utils.hpp"

Expand Down Expand Up @@ -42,9 +44,9 @@ uint64_t TimestampDictionaryWriter::get_pattern_id(TimestampPattern const* patte
}

epochtime_t TimestampDictionaryWriter::ingest_entry(
std::string const& key,
std::string_view key,
int32_t node_id,
std::string const& timestamp,
std::string_view timestamp,
uint64_t& pattern_id
) {
epochtime_t ret;
Expand Down Expand Up @@ -88,7 +90,7 @@ epochtime_t TimestampDictionaryWriter::ingest_entry(
}

void TimestampDictionaryWriter::ingest_entry(
std::string const& key,
std::string_view key,
int32_t node_id,
double timestamp
) {
Expand All @@ -103,7 +105,7 @@ void TimestampDictionaryWriter::ingest_entry(
}

void TimestampDictionaryWriter::ingest_entry(
std::string const& key,
std::string_view key,
int32_t node_id,
int64_t timestamp
) {
Expand Down
10 changes: 6 additions & 4 deletions components/core/src/clp_s/TimestampDictionaryWriter.hpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#ifndef CLP_S_TIMESTAMPDICTIONARYWRITER_HPP
#define CLP_S_TIMESTAMPDICTIONARYWRITER_HPP

#include <cstdint>
#include <map>
#include <sstream>
#include <string>
#include <string_view>
#include <unordered_map>
#include <utility>

Expand Down Expand Up @@ -47,9 +49,9 @@ class TimestampDictionaryWriter {
* @return the epoch time corresponding to the string timestamp
*/
epochtime_t ingest_entry(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not mandatory, but since it seems like a small change, do you want to change the callers in ArchiveWriter to also use string_views?

std::string const& key,
std::string_view key,
int32_t node_id,
std::string const& timestamp,
std::string_view timestamp,
uint64_t& pattern_id
);

Expand All @@ -59,9 +61,9 @@ class TimestampDictionaryWriter {
* @param node_id
* @param timestamp
*/
void ingest_entry(std::string const& key, int32_t node_id, double timestamp);
void ingest_entry(std::string_view key, int32_t node_id, double timestamp);

void ingest_entry(std::string const& key, int32_t node_id, int64_t timestamp);
void ingest_entry(std::string_view key, int32_t node_id, int64_t timestamp);

/**
* TODO: guarantee epoch milliseconds. The current clp-s approach to encoding timestamps and
Expand Down
3 changes: 2 additions & 1 deletion components/core/src/clp_s/TimestampEntry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <sstream>
#include <string>
#include <string_view>
#include <unordered_set>
#include <variant>

Expand Down Expand Up @@ -43,7 +44,7 @@ class TimestampEntry {
m_epoch_start(cEpochTimeMax),
m_epoch_end(cEpochTimeMin) {}

TimestampEntry(std::string const& key_name)
TimestampEntry(std::string_view key_name)
: m_encoding(UnkownTimestampEncoding),
m_epoch_start_double(cDoubleEpochTimeMax),
m_epoch_end_double(cDoubleEpochTimeMin),
Expand Down
28 changes: 14 additions & 14 deletions components/core/src/clp_s/TimestampPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include <chrono>
#include <cstring>
#include <string>
#include <string_view>
gibber9809 marked this conversation as resolved.
Show resolved Hide resolved
#include <vector>

#include <date/include/date/date.h>
Expand All @@ -12,6 +14,7 @@

using clp::string_utils::convert_string_to_int;
using std::string;
using std::string_view;
using std::to_string;
using std::vector;

Expand Down Expand Up @@ -71,7 +74,7 @@ append_padded_value_notz(int value, char padding_character, size_t max_length, s
* @return true if conversion succeeds, false otherwise
*/
static bool convert_string_to_number(
string const& str,
string_view str,
size_t begin_ix,
size_t end_ix,
char padding_character,
Expand All @@ -89,7 +92,7 @@ static bool convert_string_to_number(
* @return true if conversion succeeds, false otherwise
*/
static bool convert_string_to_number_notz(
string const& str,
string_view str,
size_t max_digits,
size_t begin_ix,
size_t& end_ix,
Expand Down Expand Up @@ -125,7 +128,7 @@ append_padded_value_notz(int value, char padding_character, size_t max_length, s
}

static bool convert_string_to_number(
string const& str,
string_view str,
size_t begin_ix,
size_t end_ix,
char padding_character,
Expand Down Expand Up @@ -154,7 +157,7 @@ static bool convert_string_to_number(
}

static bool convert_string_to_number_notz(
string const& str,
string_view str,
size_t max_digits,
size_t begin_ix,
size_t& end_ix,
Expand Down Expand Up @@ -306,7 +309,7 @@ void TimestampPattern::init() {
}

TimestampPattern const* TimestampPattern::search_known_ts_patterns(
string const& line,
string_view line,
epochtime_t& timestamp,
size_t& timestamp_begin_pos,
size_t& timestamp_end_pos
Expand Down Expand Up @@ -342,7 +345,7 @@ void TimestampPattern::clear() {
}

bool TimestampPattern::parse_timestamp(
string const& line,
string_view line,
epochtime_t& timestamp,
size_t& timestamp_begin_pos,
size_t& timestamp_end_pos
Expand Down Expand Up @@ -827,23 +830,20 @@ bool TimestampPattern::parse_timestamp(
}
auto dot_position = line.find('.');
auto nanosecond_start = dot_position + 1;
if (std::string::npos == dot_position || 0 == dot_position
if (string::npos == dot_position || 0 == dot_position
|| cNanosecondDigits != (line.length() - nanosecond_start))
{
return false;
}

auto timestamp_view = std::string_view(line);
if (false
== convert_string_to_int(timestamp_view.substr(0, dot_position), timestamp))
{
if (false == convert_string_to_int(line.substr(0, dot_position), timestamp)) {
return false;
}

epochtime_t timestamp_nanoseconds;
if (false
== convert_string_to_int(
timestamp_view.substr(nanosecond_start, cNanosecondDigits),
line.substr(nanosecond_start, cNanosecondDigits),
timestamp_nanoseconds
))
{
Expand Down Expand Up @@ -1070,14 +1070,14 @@ void TimestampPattern::insert_formatted_timestamp(epochtime_t timestamp, string&
case 'E': // UNIX epoch milliseconds
// Note: this timestamp format is required to make up the entire timestamp, so
// this is safe
new_msg = std::to_string(timestamp);
new_msg = to_string(timestamp);
break;

case 'F': { // Nanosecond precision floating point UNIX epoch timestamp
constexpr auto cNanosecondDigits = 9;
// Note: this timestamp format is required to make up the entire timestamp, so
// this is safe
new_msg = std::to_string(timestamp);
new_msg = to_string(timestamp);
new_msg.insert(new_msg.end() - cNanosecondDigits, '.');
break;
}
Expand Down
6 changes: 4 additions & 2 deletions components/core/src/clp_s/TimestampPattern.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <cstddef>
#include <cstdint>
#include <memory>
#include <string>
#include <string_view>
gibber9809 marked this conversation as resolved.
Show resolved Hide resolved
#include <utility>

#include "Defs.hpp"
Expand Down Expand Up @@ -83,7 +85,7 @@ class TimestampPattern {
* @return pointer to the timestamp pattern if found, nullptr otherwise
*/
static TimestampPattern const* search_known_ts_patterns(
std::string const& line,
std::string_view line,
epochtime_t& timestamp,
size_t& timestamp_begin_pos,
size_t& timestamp_end_pos
Expand Down Expand Up @@ -121,7 +123,7 @@ class TimestampPattern {
* @return true if parsed successfully, false otherwise
*/
bool parse_timestamp(
std::string const& line,
std::string_view line,
epochtime_t& timestamp,
size_t& timestamp_begin_pos,
size_t& timestamp_end_pos
Expand Down
16 changes: 3 additions & 13 deletions components/core/src/clp_s/search/StringLiteral.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <memory>
#include <string>

#include "../Utils.hpp"
#include "Literal.hpp"

namespace clp_s::search {
Expand Down Expand Up @@ -68,19 +69,8 @@ class StringLiteral : public Literal {
m_string_type = LiteralType::VarStringT;
}

// If '?' and '*' are not escaped, we add LiteralType::ClpStringT to m_string_type
bool escape = false;
for (char const c : m_v) {
if ('\\' == c) {
escape = !escape;
} else if ('?' == c || '*' == c) {
if (false == escape) {
m_string_type |= LiteralType::ClpStringT;
break;
}
} else {
escape = false;
}
if (StringUtils::has_unescaped_wildcards(m_v)) {
m_string_type |= LiteralType::ClpStringT;
}
}
};
Expand Down
Loading