Skip to content

Commit 4e0c48b

Browse files
committed
Revert "[libc++] Speed up classic locale (#72112)"
Looks like it broke the ASAN build: https://lab.llvm.org/buildbot/#/builders/168/builds/17053/steps/9/logs/stdio This reverts commit f8afc53.
1 parent 983a275 commit 4e0c48b

File tree

6 files changed

+55
-164
lines changed

6 files changed

+55
-164
lines changed
Lines changed: 3 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
#include "benchmark/benchmark.h"
22
#include "test_macros.h"
33

4-
#include <mutex>
54
#include <sstream>
65

76
TEST_NOINLINE double istream_numbers();
87

9-
double istream_numbers(std::locale* loc) {
8+
double istream_numbers() {
109
const char* a[] = {"-6 69 -71 2.4882e-02 -100 101 -2.00005 5000000 -50000000",
1110
"-25 71 7 -9.3262e+01 -100 101 -2.00005 5000000 -50000000",
1211
"-14 53 46 -6.7026e-02 -100 101 -2.00005 5000000 -50000000"};
@@ -15,73 +14,17 @@ double istream_numbers(std::locale* loc) {
1514
double f1 = 0.0, f2 = 0.0, q = 0.0;
1615
for (int i = 0; i < 3; i++) {
1716
std::istringstream s(a[i]);
18-
if (loc)
19-
s.imbue(*loc);
2017
s >> a1 >> a2 >> a3 >> f1 >> a4 >> a5 >> f2 >> a6 >> a7;
2118
q += (a1 + a2 + a3 + a4 + a5 + a6 + a7 + f1 + f2) / 1000000;
2219
}
2320
return q;
2421
}
2522

26-
struct LocaleSelector {
27-
std::locale* imbue;
28-
std::locale old;
29-
static std::mutex mutex;
30-
31-
LocaleSelector(benchmark::State& state) {
32-
std::lock_guard guard(mutex);
33-
switch (state.range(0)) {
34-
case 0: {
35-
old = std::locale::global(std::locale::classic());
36-
imbue = nullptr;
37-
break;
38-
}
39-
case 1: {
40-
old = std::locale::global(std::locale::classic());
41-
thread_local std::locale loc("en_US.UTF-8");
42-
imbue = &loc;
43-
break;
44-
}
45-
case 2: {
46-
old = std::locale::global(std::locale::classic());
47-
static std::locale loc("en_US.UTF-8");
48-
imbue = &loc;
49-
break;
50-
}
51-
case 3: {
52-
old = std::locale::global(std::locale("en_US.UTF-8"));
53-
imbue = nullptr;
54-
break;
55-
}
56-
}
57-
}
58-
59-
~LocaleSelector() {
60-
std::lock_guard guard(mutex);
61-
std::locale::global(old);
62-
}
63-
};
64-
65-
std::mutex LocaleSelector::mutex;
66-
6723
static void BM_Istream_numbers(benchmark::State& state) {
68-
LocaleSelector sel(state);
6924
double i = 0;
7025
while (state.KeepRunning())
71-
benchmark::DoNotOptimize(i += istream_numbers(sel.imbue));
72-
}
73-
BENCHMARK(BM_Istream_numbers)->DenseRange(0, 3)->UseRealTime()->Threads(1)->ThreadPerCpu();
74-
75-
static void BM_Ostream_number(benchmark::State& state) {
76-
LocaleSelector sel(state);
77-
while (state.KeepRunning()) {
78-
std::ostringstream ss;
79-
if (sel.imbue)
80-
ss.imbue(*sel.imbue);
81-
ss << 0;
82-
benchmark::DoNotOptimize(ss.str().c_str());
83-
}
26+
benchmark::DoNotOptimize(i += istream_numbers());
8427
}
85-
BENCHMARK(BM_Ostream_number)->DenseRange(0, 3)->UseRealTime()->Threads(1)->ThreadPerCpu();
8628

29+
BENCHMARK(BM_Istream_numbers)->RangeMultiplier(2)->Range(1024, 4096);
8730
BENCHMARK_MAIN();

libcxx/include/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,6 @@ set(files
849849
__utility/integer_sequence.h
850850
__utility/is_pointer_in_range.h
851851
__utility/move.h
852-
__utility/no_destroy.h
853852
__utility/pair.h
854853
__utility/piecewise_construct.h
855854
__utility/priority_tag.h

libcxx/include/__locale

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include <__memory/shared_ptr.h> // __shared_count
1616
#include <__mutex/once_flag.h>
1717
#include <__type_traits/make_unsigned.h>
18-
#include <__utility/no_destroy.h>
1918
#include <cctype>
2019
#include <clocale>
2120
#include <cstdint>

libcxx/include/__utility/no_destroy.h

Lines changed: 0 additions & 56 deletions
This file was deleted.

libcxx/include/module.modulemap.in

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2042,7 +2042,6 @@ module std_private_utility_move [system] {
20422042
export std_private_type_traits_is_nothrow_move_constructible
20432043
export std_private_type_traits_remove_reference
20442044
}
2045-
module std_private_utility_no_destroy [system] { header "__utility/no_destroy.h" }
20462045
module std_private_utility_pair [system] {
20472046
header "__utility/pair.h"
20482047
export std_private_ranges_subrange_fwd

libcxx/src/locale.cpp

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include <__utility/no_destroy.h>
109
#include <algorithm>
1110
#include <clocale>
1211
#include <codecvt>
@@ -82,8 +81,9 @@ locale_t __cloc() {
8281

8382
namespace {
8483

85-
struct releaser {
86-
void operator()(locale::facet* p) { p->__release_shared(); }
84+
struct release
85+
{
86+
void operator()(locale::facet* p) {p->__release_shared();}
8787
};
8888

8989
template <class T, class ...Args>
@@ -155,11 +155,7 @@ class _LIBCPP_HIDDEN locale::__imp
155155
{return static_cast<size_t>(id) < facets_.size() && facets_[static_cast<size_t>(id)];}
156156
const locale::facet* use_facet(long id) const;
157157

158-
void acquire();
159-
void release();
160-
static __no_destroy<__imp> classic_locale_imp_;
161-
162-
private:
158+
private:
163159
void install(facet* f, long id);
164160
template <class F> void install(F* f) {install(f, f->id.__get());}
165161
template <class F> void install_from(const __imp& other);
@@ -504,7 +500,7 @@ locale::__imp::__imp(const __imp& other, facet* f, long id)
504500
name_("*")
505501
{
506502
f->__add_shared();
507-
unique_ptr<facet, releaser> hold(f);
503+
unique_ptr<facet, release> hold(f);
508504
facets_ = other.facets_;
509505
for (unsigned i = 0; i < other.facets_.size(); ++i)
510506
if (facets_[i])
@@ -523,7 +519,7 @@ void
523519
locale::__imp::install(facet* f, long id)
524520
{
525521
f->__add_shared();
526-
unique_ptr<facet, releaser> hold(f);
522+
unique_ptr<facet, release> hold(f);
527523
if (static_cast<size_t>(id) >= facets_.size())
528524
facets_.resize(static_cast<size_t>(id+1));
529525
if (facets_[static_cast<size_t>(id)])
@@ -541,78 +537,89 @@ locale::__imp::use_facet(long id) const
541537

542538
// locale
543539

544-
// We don't do reference counting on the classic locale.
545-
// It's never destroyed anyway, but atomic reference counting may be very
546-
// expensive in parallel applications. The classic locale is used by default
547-
// in all streams. Note: if a new global locale is installed, then we lose
548-
// the benefit of no reference counting.
549-
__no_destroy<locale::__imp> locale::__imp::classic_locale_imp_(__uninitialized_tag{}); // initialized below in classic()
540+
// This class basically implements __attribute__((no_destroy)), which isn't supported
541+
// by GCC as of writing this.
542+
template <class T>
543+
struct __no_destroy {
544+
template <class... Args>
545+
explicit __no_destroy(Args&&... args) {
546+
T* obj = reinterpret_cast<T*>(&buf);
547+
new (obj) T(std::forward<Args>(args)...);
548+
}
549+
550+
T& get() { return *reinterpret_cast<T*>(&buf); }
551+
T const& get() const { return *reinterpret_cast<T const*>(&buf); }
552+
553+
private:
554+
alignas(T) byte buf[sizeof(T)];
555+
};
550556

551557
const locale& locale::classic() {
552-
static const __no_destroy<locale> classic_locale(__private_tag{}, [] {
553-
// executed exactly once on first initialization of `classic_locale`
554-
locale::__imp::classic_locale_imp_.__emplace(1u);
555-
return &locale::__imp::classic_locale_imp_.__get();
556-
}());
557-
return classic_locale.__get();
558+
static const __no_destroy<locale> c(__private_tag{}, &make<__imp>(1u));
559+
return c.get();
558560
}
559561

560562
locale& locale::__global() {
561-
static __no_destroy<locale> g(locale::classic());
562-
return g.__get();
563+
static __no_destroy<locale> g(locale::classic());
564+
return g.get();
563565
}
564566

565-
void locale::__imp::acquire() {
566-
if (this != &locale::__imp::classic_locale_imp_.__get())
567-
__add_shared();
567+
locale::locale() noexcept
568+
: __locale_(__global().__locale_)
569+
{
570+
__locale_->__add_shared();
568571
}
569572

570-
void locale::__imp::release() {
571-
if (this != &locale::__imp::classic_locale_imp_.__get())
572-
__release_shared();
573+
locale::locale(const locale& l) noexcept
574+
: __locale_(l.__locale_)
575+
{
576+
__locale_->__add_shared();
573577
}
574578

575-
locale::locale() noexcept : __locale_(__global().__locale_) { __locale_->acquire(); }
576-
577-
locale::locale(const locale& l) noexcept : __locale_(l.__locale_) { __locale_->acquire(); }
578-
579-
locale::~locale() { __locale_->release(); }
579+
locale::~locale()
580+
{
581+
__locale_->__release_shared();
582+
}
580583

581584
const locale&
582585
locale::operator=(const locale& other) noexcept
583586
{
584-
other.__locale_->acquire();
585-
__locale_->release();
586-
__locale_ = other.__locale_;
587-
return *this;
587+
other.__locale_->__add_shared();
588+
__locale_->__release_shared();
589+
__locale_ = other.__locale_;
590+
return *this;
588591
}
589592

590593
locale::locale(const char* name)
591594
: __locale_(name ? new __imp(name)
592595
: (__throw_runtime_error("locale constructed with null"), nullptr))
593596
{
594-
__locale_->acquire();
597+
__locale_->__add_shared();
595598
}
596599

597-
locale::locale(const string& name) : __locale_(new __imp(name)) { __locale_->acquire(); }
600+
locale::locale(const string& name)
601+
: __locale_(new __imp(name))
602+
{
603+
__locale_->__add_shared();
604+
}
598605

599606
locale::locale(const locale& other, const char* name, category c)
600607
: __locale_(name ? new __imp(*other.__locale_, name, c)
601608
: (__throw_runtime_error("locale constructed with null"), nullptr))
602609
{
603-
__locale_->acquire();
610+
__locale_->__add_shared();
604611
}
605612

606613
locale::locale(const locale& other, const string& name, category c)
607614
: __locale_(new __imp(*other.__locale_, name, c))
608615
{
609-
__locale_->acquire();
616+
__locale_->__add_shared();
610617
}
611618

612619
locale::locale(const locale& other, const locale& one, category c)
613620
: __locale_(new __imp(*other.__locale_, *one.__locale_, c))
614621
{
615-
__locale_->acquire();
622+
__locale_->__add_shared();
616623
}
617624

618625
string
@@ -628,7 +635,7 @@ locale::__install_ctor(const locale& other, facet* f, long id)
628635
__locale_ = new __imp(*other.__locale_, f, id);
629636
else
630637
__locale_ = other.__locale_;
631-
__locale_->acquire();
638+
__locale_->__add_shared();
632639
}
633640

634641
locale

0 commit comments

Comments
 (0)