Skip to content

Commit 1018d87

Browse files
committed
Context improvements
- prevent a superfluous copy of ContextValue - replace raw char* key pointer with std::string, increases sizeof(DataList) but introduces small string optimization - introduced rule of 0 as far as possible (copy/move constructor and assignment and also destructor can be generated by compiler)
1 parent b568e71 commit 1018d87

File tree

1 file changed

+12
-55
lines changed

1 file changed

+12
-55
lines changed

api/include/opentelemetry/context/context.h

Lines changed: 12 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ class Context
4444
template <class T>
4545
Context SetValues(T &values) noexcept
4646
{
47-
Context context = Context(values);
48-
nostd::shared_ptr<DataList> last = context.head_;
47+
Context context(values);
48+
auto last = context.head_;
4949
while (last->next_ != nullptr)
5050
{
5151
last = last->next_;
@@ -57,7 +57,7 @@ class Context
5757
// Accepts a new iterable and then returns a new context that
5858
// contains the new key and value data. It attaches the
5959
// exisiting list to the end of the new list.
60-
Context SetValue(nostd::string_view key, ContextValue value) noexcept
60+
Context SetValue(nostd::string_view key, const ContextValue& value) noexcept
6161
{
6262
Context context = Context(key, value);
6363
context.head_->next_ = head_;
@@ -69,12 +69,9 @@ class Context
6969
{
7070
for (DataList *data = head_.get(); data != nullptr; data = data->next_.get())
7171
{
72-
if (key.size() == data->key_length_)
72+
if (key == data->key_)
7373
{
74-
if (std::memcmp(key.data(), data->key_, data->key_length_) == 0)
75-
{
76-
return data->value_;
77-
}
74+
return data->value_;
7875
}
7976
}
8077
return ContextValue{};
@@ -92,12 +89,10 @@ class Context
9289
// A linked list to contain the keys and values of this context node
9390
struct DataList
9491
{
95-
char *key_ = nullptr;
92+
std::string key_;
9693

9794
nostd::shared_ptr<DataList> next_{nullptr};
9895

99-
size_t key_length_ = 0UL;
100-
10196
ContextValue value_;
10297

10398
DataList() = default;
@@ -106,62 +101,24 @@ class Context
106101
template <class T>
107102
DataList(const T &keys_and_vals)
108103
{
109-
bool first = true;
110104
auto *node = this;
111-
for (auto &iter : keys_and_vals)
105+
auto iter = std::begin(keys_and_vals);
106+
*node = DataList(iter.first, iter.second);
107+
for (++iter; iter != std::end(keys_and_vals); ++iter)
112108
{
113-
if (first)
114-
{
115-
*node = DataList(iter.first, iter.second);
116-
first = false;
117-
}
118-
else
119-
{
120-
node->next_ = nostd::shared_ptr<DataList>(new DataList(iter.first, iter.second));
121-
node = node->next_.get();
122-
}
109+
node->next_ = nostd::shared_ptr<DataList>(new DataList(iter.first, iter.second));
110+
node = node->next_.get();
123111
}
124112
}
125113

126114
// Builds a data list with just a key and value, so it will just be the head
127115
// and returns that head.
128116
DataList(nostd::string_view key, const ContextValue &value)
129117
{
130-
key_ = new char[key.size()];
131-
key_length_ = key.size();
132-
std::memcpy(key_, key.data(), key.size() * sizeof(char));
118+
key_.assign(key.begin(), key.end());
133119
next_ = nostd::shared_ptr<DataList>{nullptr};
134120
value_ = value;
135121
}
136-
137-
DataList(const DataList &other)
138-
: key_(new char[other.key_length_]),
139-
next_(other.next_),
140-
key_length_(other.key_length_),
141-
value_(other.value_)
142-
{
143-
std::memcpy(key_, other.key_, other.key_length_ * sizeof(char));
144-
}
145-
146-
DataList &operator=(DataList &&other) noexcept
147-
{
148-
key_length_ = other.key_length_;
149-
value_ = std::move(other.value_);
150-
next_ = std::move(other.next_);
151-
152-
key_ = other.key_;
153-
other.key_ = nullptr;
154-
155-
return *this;
156-
}
157-
158-
~DataList()
159-
{
160-
if (key_ != nullptr)
161-
{
162-
delete[] key_;
163-
}
164-
}
165122
};
166123

167124
// Head of the list which holds the keys and values of this context

0 commit comments

Comments
 (0)