On Fri, Mar 19, 2021 at 2:12 PM Casey Bodley <cbodley@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