[libcxx-commits] [PATCH] D67086: Implement syncstream (p0053)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/syncstream:206
+
+  [[nodiscard]] static _LIBCPP_HIDE_FROM_ABI __wrapped_streambuf_mutex& __instance() noexcept {
+    static __wrapped_streambuf_mutex __result;
----------------
It *might* be possible to use a bit in the `locale` to implement a lock for all streambufs. Something like

```
diff --git a/libcxx/include/__locale b/libcxx/include/__locale
index ccac748c44e4..4942808ac2bc 100644
--- a/libcxx/include/__locale
+++ b/libcxx/include/__locale
@@ -131,9 +131,11 @@ private:
     static locale& __global();
     bool has_facet(id&) const;
     const facet* use_facet(id&) const;
+};
 
-    template <class _Facet> friend bool has_facet(const locale&)  _NOEXCEPT;
-    template <class _Facet> friend const _Facet& use_facet(const locale&);
+template <>
+struct __free_bit_traits<locale> {
+    // expose the free bit in `__imp*`
 };
 
 class _LIBCPP_EXPORTED_FROM_ABI locale::facet
diff --git a/libcxx/include/streambuf b/libcxx/include/streambuf
index 095ac7d3ad83..b42fae608a5a 100644
--- a/libcxx/include/streambuf
+++ b/libcxx/include/streambuf
@@ -296,7 +296,9 @@ protected:
     virtual int_type overflow(int_type __c = traits_type::eof());
 
 private:
-    locale __loc_;
+    static_assert(__has_free_bits<locale>);
+    __free_bit_pair<locale, 1> __loc_;
+    // Now implement a lock using that single free bit.
     char_type* __binp_;
     char_type* __ninp_;
     char_type* __einp_;
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index 59c7ce4d43d6..ade1aa688c4d 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -543,7 +543,7 @@ locale::__imp::make_classic()
     // only one thread can get in here and it only gets in once
     static aligned_storage<sizeof(locale)>::type buf;
     locale* c = reinterpret_cast<locale*>(&buf);
-    c->__locale_ = &make<__imp>(1u);
+    c->__get_locale() = &make<__imp>(1u);
     return *c;
 }
 
```

Do you know what other implementations have done? Have they gone through the trouble of finding a lock inside streambufs or do they use a global map?


================
Comment at: libcxx/include/syncstream:386
+
+  basic_string<_CharT, _Traits, _Allocator> __str_;
+  bool __emit_on_sync_{false};
----------------
Do we take advantage of what `std::string` provides (i.e. growth characteristics, SBO, etc..). If we do take advantage of those, then it might be worth using `std::string`, otherwise I think we might as well use a character buffer we manually manage to save on the duplication of begin/end information.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67086/new/

https://reviews.llvm.org/D67086