Skip to content

Commit

Permalink
Use template class for in-memory data. (#1496)
Browse files Browse the repository at this point in the history
  • Loading branch information
yxue authored Jul 18, 2022
1 parent fba16c6 commit 4062237
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 55 deletions.
1 change: 1 addition & 0 deletions exporters/memory/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
cc_library(
name = "in_memory_span_data",
hdrs = [
"include/opentelemetry/exporters/memory/in_memory_data.h",
"include/opentelemetry/exporters/memory/in_memory_span_data.h",
],
strip_include_prefix = "include",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#pragma once

#include "opentelemetry/sdk/common/circular_buffer.h"

#include <vector>

OPENTELEMETRY_BEGIN_NAMESPACE
namespace exporter
{
namespace memory
{
/**
* A wrapper class holding in memory exporter data
*/
template <typename T>
class InMemoryData
{
public:
/**
* @param buffer_size a required value that sets the size of the CircularBuffer
*/
InMemoryData(size_t buffer_size) : data_(buffer_size) {}

/**
* @param data a required unique pointer to the data to add to the CircularBuffer
*/
void Add(std::unique_ptr<T> data) noexcept { data_.Add(data); }

/**
* @return Returns a vector of unique pointers containing all the data in the
* CircularBuffer. This operation will empty the Buffer, which is why the data
* is returned as unique pointers
*/
std::vector<std::unique_ptr<T>> Get() noexcept
{
std::vector<std::unique_ptr<T>> res;

// Pointer swap is required because the Consume function requires that the
// AtomicUniquePointer be set to null
data_.Consume(
data_.size(), [&](opentelemetry::sdk::common::CircularBufferRange<
opentelemetry::sdk::common::AtomicUniquePtr<T>> range) noexcept {
range.ForEach([&](opentelemetry::sdk::common::AtomicUniquePtr<T> &ptr) noexcept {
std::unique_ptr<T> swap_ptr = nullptr;
ptr.Swap(swap_ptr);
res.push_back(std::move(swap_ptr));
return true;
});
});

return res;
}

private:
opentelemetry::sdk::common::CircularBuffer<T> data_;
};
} // namespace memory
} // namespace exporter
OPENTELEMETRY_END_NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

#pragma once

#include "opentelemetry/sdk/common/circular_buffer.h"
#include "opentelemetry/exporters/memory/in_memory_data.h"
#include "opentelemetry/sdk/trace/recordable.h"
#include "opentelemetry/sdk/trace/span_data.h"

Expand All @@ -14,60 +14,17 @@ namespace exporter
{
namespace memory
{
/**
* A wrapper class holding in memory exporter data
*/
class InMemorySpanData final
class InMemorySpanData final : public exporter::memory::InMemoryData<sdk::trace::SpanData>
{
public:
/**
* @param buffer_size a required value that sets the size of the CircularBuffer
*/
InMemorySpanData(size_t buffer_size) : spans_received_(buffer_size) {}
explicit InMemorySpanData(size_t buffer_size)
: exporter::memory::InMemoryData<sdk::trace::SpanData>(buffer_size)
{}

/**
* @param data a required unique pointer to the data to add to the CircularBuffer
*/
void Add(std::unique_ptr<opentelemetry::sdk::trace::SpanData> data) noexcept
{
std::unique_ptr<opentelemetry::sdk::trace::SpanData> span_data(
static_cast<opentelemetry::sdk::trace::SpanData *>(data.release()));
spans_received_.Add(span_data);
}

/**
* @return Returns a vector of unique pointers containing all the span data in the
* CircularBuffer. This operation will empty the Buffer, which is why the data
* is returned as unique pointers
*/
std::vector<std::unique_ptr<opentelemetry::sdk::trace::SpanData>> GetSpans() noexcept
{
std::vector<std::unique_ptr<opentelemetry::sdk::trace::SpanData>> res;

// Pointer swap is required because the Consume function requires that the
// AtomicUniquePointer be set to null
spans_received_.Consume(
spans_received_.size(),
[&](opentelemetry::sdk::common::CircularBufferRange<
opentelemetry::sdk::common::AtomicUniquePtr<opentelemetry::sdk::trace::SpanData>>
range) noexcept {
range.ForEach(
[&](opentelemetry::sdk::common::AtomicUniquePtr<opentelemetry::sdk::trace::SpanData>
&ptr) noexcept {
std::unique_ptr<opentelemetry::sdk::trace::SpanData> swap_ptr =
std::unique_ptr<opentelemetry::sdk::trace::SpanData>(nullptr);
ptr.Swap(swap_ptr);
res.push_back(
std::unique_ptr<opentelemetry::sdk::trace::SpanData>(swap_ptr.release()));
return true;
});
});

return res;
}

private:
opentelemetry::sdk::common::CircularBuffer<opentelemetry::sdk::trace::SpanData> spans_received_;
std::vector<std::unique_ptr<sdk::trace::SpanData>> GetSpans() noexcept { return Get(); }
};
} // namespace memory
} // namespace exporter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "opentelemetry/common/spin_lock_mutex.h"
#include "opentelemetry/exporters/memory/in_memory_span_data.h"
#include "opentelemetry/sdk/trace/exporter.h"
#include "opentelemetry/sdk/trace/span_data.h"
#include "opentelemetry/sdk_config.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand All @@ -26,7 +27,7 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
* @param buffer_size an optional value that sets the size of the InMemorySpanData
*/
InMemorySpanExporter(size_t buffer_size = MAX_BUFFER_SIZE)
: data_(new opentelemetry::exporter::memory::InMemorySpanData(buffer_size))
: data_(new InMemorySpanData(buffer_size))
{}

/**
Expand Down Expand Up @@ -80,13 +81,10 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
/**
* @return Returns a shared pointer to this exporters InMemorySpanData
*/
std::shared_ptr<opentelemetry::exporter::memory::InMemorySpanData> GetData() noexcept
{
return data_;
}
std::shared_ptr<InMemorySpanData> GetData() noexcept { return data_; }

private:
std::shared_ptr<opentelemetry::exporter::memory::InMemorySpanData> data_;
std::shared_ptr<InMemorySpanData> data_;
bool is_shutdown_ = false;
mutable opentelemetry::common::SpinLockMutex lock_;
const bool isShutdown() const noexcept
Expand Down

2 comments on commit 4062237

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 4062237 Previous: fba16c6 Ratio
BM_AttributseHashMap 23119664.192199707 ns/iter 10891712.62887808 ns/iter 2.12
BM_LockFreeBuffer/1 777603.8646697998 ns/iter 295930.403795066 ns/iter 2.63
BM_LockFreeBuffer/2 1983299.0169525146 ns/iter 986895.5612182617 ns/iter 2.01

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 4062237 Previous: fba16c6 Ratio
BM_SpinLockThrashing/1/process_time/real_time 0.6769353693181818 ms/iter 0.12063959513715306 ms/iter 5.61
BM_SpinLockThrashing/2/process_time/real_time 1.6767105586092237 ms/iter 0.23872146801072724 ms/iter 7.02
BM_ProcYieldSpinLockThrashing/1/process_time/real_time 0.5251633698673924 ms/iter 0.12088441766114127 ms/iter 4.34
BM_ProcYieldSpinLockThrashing/2/process_time/real_time 1.4159321784973145 ms/iter 0.24126741711488167 ms/iter 5.87
BM_NaiveSpinLockThrashing/1/process_time/real_time 0.7165804911216409 ms/iter 0.12069827718291554 ms/iter 5.94
BM_NaiveSpinLockThrashing/2/process_time/real_time 2.9533360455487228 ms/iter 0.23896458205916368 ms/iter 12.36
BM_ThreadYieldSpinLockThrashing/1/process_time/real_time 38.23590278625488 ms/iter 8.930504322052002 ms/iter 4.28

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.