On Fri, Mar 19, 2021 at 2:12 PM Casey Bodley <cbodley(a)redhat.com> wrote:
hi Kefu,
continuing our discussion from
https://github.com/ceph/ceph/pull/40230
on the future of this BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT define
to summarize the issue:
in 1.66, boost::asio made a lot of changes for 'Networking TS
compatibility', including the executors proposed therein. i raised
this on ceph-devel in the thread "coming in boost 1.66" (see
https://www.spinics.net/lists/ceph-devel/msg39243.html)
meanwhile, the c++ standards committee was working on 'unified
executors' proposals outside of the Networking TS, and networking was
left out of c++20 so it could wait for a unified executor model
instead of adding its own
in 1.74, boost::asio added support for this new executor model, which
its docs summarize well at
https://www.boost.org/doc/libs/1_74_0/doc/html/boost_asio/std_executors.html.
a BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT option was added to preserve
compatibility with existing code, so ceph now relies on this in
several places to build against boost 1.74+
i've been hesitant to push for a conversion to this new model for two
main reasons:
* it's mostly internal to asio, so i don't see much benefit to
changing as long as boost continues to support the TS executors
* it's hard to tell how close it is to the 'final form' that we'll see
in a future c++ standard, so later changes may require us to do
another conversion
does anyone else have a stake in this? if there's interest in working
on it, i'm happy to help with review
I'd vote for at least just being consistent since it's an
internal-only change. Right now, "src/rgw/CMakeLists.txt" has
"add_definitions(-DBOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT)" (via commit
722b4303b198) and so does "src/librbd/CMakeList.txt" (via commit
3d708219092d). Since the core code is compiled w/ the new TS executor
enabled but the tests don't seem to include that, maybe we just need
to make it global.
--
Jason