Skip to content

Commit

Permalink
Fix header only api singletons (#1520) (#1604)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcalff authored Sep 13, 2022
1 parent cfaf8a1 commit 74f0ac1
Show file tree
Hide file tree
Showing 26 changed files with 900 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Increment the:
* [METRICS EXPORTER] Add `OtlpGrpcMetricExporterFactory` and `OtlpHttpMetricExporterFactory`.
[#1606](https://github.com/open-telemetry/opentelemetry-cpp/pull/1606)
* [METRICS EXPORTER] Add `OtlpGrpcClient` [#1606](https://github.com/open-telemetry/opentelemetry-cpp/pull/1606)
* [BUILD] Fix header only api singletons [#1604](https://github.com/open-telemetry/opentelemetry-cpp/pull/1604)

## [1.6.0] 2022-08-15

Expand Down
5 changes: 3 additions & 2 deletions api/include/opentelemetry/_metrics/provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

# include "opentelemetry/_metrics/meter_provider.h"
# include "opentelemetry/_metrics/noop.h"
# include "opentelemetry/common/macros.h"
# include "opentelemetry/common/spin_lock_mutex.h"
# include "opentelemetry/nostd/shared_ptr.h"

Expand Down Expand Up @@ -41,13 +42,13 @@ class Provider
}

private:
static nostd::shared_ptr<MeterProvider> &GetProvider() noexcept
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<MeterProvider> &GetProvider() noexcept
{
static nostd::shared_ptr<MeterProvider> provider(new NoopMeterProvider);
return provider;
}

static common::SpinLockMutex &GetLock() noexcept
OPENTELEMETRY_API_SINGLETON static common::SpinLockMutex &GetLock() noexcept
{
static common::SpinLockMutex lock;
return lock;
Expand Down
3 changes: 2 additions & 1 deletion api/include/opentelemetry/baggage/baggage.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <cctype>

#include "opentelemetry/common/kv_properties.h"
#include "opentelemetry/common/macros.h"
#include "opentelemetry/nostd/shared_ptr.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/version.h"
Expand Down Expand Up @@ -34,7 +35,7 @@ class Baggage
: kv_properties_(new opentelemetry::common::KeyValueProperties(keys_and_values))
{}

static nostd::shared_ptr<Baggage> GetDefault()
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<Baggage> GetDefault()
{
static nostd::shared_ptr<Baggage> baggage{new Baggage()};
return baggage;
Expand Down
83 changes: 83 additions & 0 deletions api/include/opentelemetry/common/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,86 @@
#else
# define OPENTELEMETRY_DEPRECATED_MESSAGE(msg)
#endif

/* clang-format off */

/**
@page HEADER_ONLY_SINGLETON Header only singleton.
@section ELF_SINGLETON
For clang and gcc, the desired coding pattern is as follows.
@verbatim
class Foo
{
// (a)
__attribute__((visibility("default")))
// (b)
T& get_singleton()
{
// (c)
static T singleton;
return singleton;
}
};
@endverbatim
(a) is needed when the code is build with
@code -fvisibility="hidden" @endcode
to ensure that all instances of (b) are visible to the linker.
What is duplicated in the binary is @em code, in (b).
The linker will make sure only one instance
of all the (b) methods is used.
(c) is a singleton implemented inside a method.
This is very desirable, because:
- the C++ compiler guarantees that construction
of the variable (c) is thread safe.
- constructors for (c) singletons are executed in code path order,
or not at all if the singleton is never used.
@section OTHER_SINGLETON
For other platforms, header only singletons are not supported at this
point.
@section CODING_PATTERN
The coding pattern to use in the source code is as follows
@verbatim
class Foo
{
OPENTELEMETRY_API_SINGLETON
T& get_singleton()
{
static T singleton;
return singleton;
}
};
@endverbatim
*/

/* clang-format on */

#if defined(__clang__)

# define OPENTELEMETRY_API_SINGLETON __attribute__((visibility("default")))

#elif defined(__GNUC__)

# define OPENTELEMETRY_API_SINGLETON __attribute__((visibility("default")))

#else

/* Add support for other compilers here. */

# define OPENTELEMETRY_API_SINGLETON

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "opentelemetry/context/propagation/noop_propagator.h"
#include "opentelemetry/context/propagation/text_map_propagator.h"

#include "opentelemetry/common/macros.h"
#include "opentelemetry/common/spin_lock_mutex.h"
#include "opentelemetry/nostd/shared_ptr.h"

Expand Down Expand Up @@ -37,13 +38,13 @@ class GlobalTextMapPropagator
}

private:
static nostd::shared_ptr<TextMapPropagator> &GetPropagator() noexcept
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<TextMapPropagator> &GetPropagator() noexcept
{
static nostd::shared_ptr<TextMapPropagator> propagator(new NoOpPropagator());
return propagator;
}

static common::SpinLockMutex &GetLock() noexcept
OPENTELEMETRY_API_SINGLETON static common::SpinLockMutex &GetLock() noexcept
{
static common::SpinLockMutex lock;
return lock;
Expand All @@ -52,4 +53,4 @@ class GlobalTextMapPropagator

} // namespace propagation
} // namespace context
OPENTELEMETRY_END_NAMESPACE
OPENTELEMETRY_END_NAMESPACE
5 changes: 3 additions & 2 deletions api/include/opentelemetry/context/runtime_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#pragma once

#include "opentelemetry/common/macros.h"
#include "opentelemetry/context/context.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand Down Expand Up @@ -166,7 +167,7 @@ class RuntimeContext
return GetStorage();
}

static nostd::shared_ptr<RuntimeContextStorage> &GetStorage() noexcept
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<RuntimeContextStorage> &GetStorage() noexcept
{
static nostd::shared_ptr<RuntimeContextStorage> context(GetDefaultStorage());
return context;
Expand Down Expand Up @@ -315,7 +316,7 @@ class ThreadLocalContextStorage : public RuntimeContextStorage
Context *base_;
};

Stack &GetStack()
OPENTELEMETRY_API_SINGLETON Stack &GetStack()
{
static thread_local Stack stack_ = Stack();
return stack_;
Expand Down
5 changes: 3 additions & 2 deletions api/include/opentelemetry/logs/provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# include <mutex>

# include "opentelemetry/common/macros.h"
# include "opentelemetry/common/spin_lock_mutex.h"
# include "opentelemetry/logs/logger_provider.h"
# include "opentelemetry/logs/noop.h"
Expand Down Expand Up @@ -42,13 +43,13 @@ class Provider
}

private:
static nostd::shared_ptr<LoggerProvider> &GetProvider() noexcept
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<LoggerProvider> &GetProvider() noexcept
{
static nostd::shared_ptr<LoggerProvider> provider(new NoopLoggerProvider);
return provider;
}

static common::SpinLockMutex &GetLock() noexcept
OPENTELEMETRY_API_SINGLETON static common::SpinLockMutex &GetLock() noexcept
{
static common::SpinLockMutex lock;
return lock;
Expand Down
7 changes: 4 additions & 3 deletions api/include/opentelemetry/metrics/provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# include <mutex>

# include "opentelemetry/common/macros.h"
# include "opentelemetry/common/spin_lock_mutex.h"
# include "opentelemetry/metrics/meter_provider.h"
# include "opentelemetry/metrics/noop.h"
Expand Down Expand Up @@ -42,13 +43,13 @@ class Provider
}

private:
static nostd::shared_ptr<MeterProvider> &GetProvider() noexcept
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<MeterProvider> &GetProvider() noexcept
{
static nostd::shared_ptr<MeterProvider> provider(new NoopMeterProvider);
return provider;
}

static common::SpinLockMutex &GetLock() noexcept
OPENTELEMETRY_API_SINGLETON static common::SpinLockMutex &GetLock() noexcept
{
static common::SpinLockMutex lock;
return lock;
Expand All @@ -57,4 +58,4 @@ class Provider

} // namespace metrics
OPENTELEMETRY_END_NAMESPACE
#endif
#endif
5 changes: 3 additions & 2 deletions api/include/opentelemetry/trace/provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <mutex>

#include "opentelemetry/common/macros.h"
#include "opentelemetry/common/spin_lock_mutex.h"
#include "opentelemetry/nostd/shared_ptr.h"
#include "opentelemetry/trace/noop.h"
Expand Down Expand Up @@ -41,13 +42,13 @@ class Provider
}

private:
static nostd::shared_ptr<TracerProvider> &GetProvider() noexcept
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<TracerProvider> &GetProvider() noexcept
{
static nostd::shared_ptr<TracerProvider> provider(new NoopTracerProvider);
return provider;
}

static common::SpinLockMutex &GetLock() noexcept
OPENTELEMETRY_API_SINGLETON static common::SpinLockMutex &GetLock() noexcept
{
static common::SpinLockMutex lock;
return lock;
Expand Down
4 changes: 3 additions & 1 deletion api/include/opentelemetry/trace/trace_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#endif

#include "opentelemetry/common/kv_properties.h"
#include "opentelemetry/common/macros.h"
#include "opentelemetry/nostd/shared_ptr.h"
#include "opentelemetry/nostd/span.h"
#include "opentelemetry/nostd/string_view.h"
Expand All @@ -41,7 +42,7 @@ class TraceState
static constexpr auto kKeyValueSeparator = '=';
static constexpr auto kMembersSeparator = ',';

static nostd::shared_ptr<TraceState> GetDefault()
OPENTELEMETRY_API_SINGLETON static nostd::shared_ptr<TraceState> GetDefault()
{
static nostd::shared_ptr<TraceState> ts{new TraceState()};
return ts;
Expand Down Expand Up @@ -316,5 +317,6 @@ class TraceState
// Store entries in a C-style array to avoid using std::array or std::vector.
nostd::unique_ptr<opentelemetry::common::KeyValueProperties> kv_properties_;
};

} // namespace trace
OPENTELEMETRY_END_NAMESPACE
1 change: 1 addition & 0 deletions api/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ if(WITH_LOGS_PREVIEW)
endif()
add_subdirectory(common)
add_subdirectory(baggage)
add_subdirectory(singleton)
Loading

2 comments on commit 74f0ac1

@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: 74f0ac1 Previous: cfaf8a1 Ratio
BM_SpinLockThrashing/1/process_time/real_time 7.333413759867351 ms/iter 0.12246077520805493 ms/iter 59.88
BM_ProcYieldSpinLockThrashing/1/process_time/real_time 0.40672819710633146 ms/iter 0.12079676334295748 ms/iter 3.37
BM_ProcYieldSpinLockThrashing/2/process_time/real_time 0.7140072909268466 ms/iter 0.23837843719793825 ms/iter 3.00
BM_NaiveSpinLockThrashing/1/process_time/real_time 0.2750284480352471 ms/iter 0.12073693452058015 ms/iter 2.28
BM_NaiveSpinLockThrashing/2/process_time/real_time 0.6032542372302199 ms/iter 0.2381017707681737 ms/iter 2.53

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 sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 74f0ac1 Previous: cfaf8a1 Ratio
BM_BaselineBuffer/1 18378808.49838257 ns/iter 5338163.375854492 ns/iter 3.44
BM_LockFreeBuffer/1 4376657.962799072 ns/iter 296720.69335845846 ns/iter 14.75
BM_LockFreeBuffer/2 4314862.966537476 ns/iter 2043425.1220054582 ns/iter 2.11

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

Please sign in to comment.