I explain this by example (then I'll speak more generally). Native_socket_stream_cfg:: has two key constexpr flags; for our purposes here, just for discussion, we can consider these one flag X; just assume for now both component flags are either true or false.
X = S_USE_OS_DGRAM_SUPPORT = S_USE_OS_DGRAM_BATCH_SUPPORT
If X=true, then Native_socket_stream will use Unix domain sockets in SOCK_SEQPACKET mode, and it'll use receive-batching via recvmmsg() (note two ms) if asked to batch-receive; furthermore struc::Channel will auto-use a batching code path.
If X=false, then N_s_s will use the classic SOCK_STREAM mode instead, which is in our case slower and trickier; and receive-batching just internally uses "emulation" (simply combines 1-message receives); furthermore struc::Channel will revert to a non-batching code path.
Key point 1: This dichotomy is compile-time. The path-not-taken is not even compiled in most cases; and though a couple nooks and crannies are compiled due to certain C++ technicalities, it is never called by anything (possibly optimized away in the end... but at least a compile error would be detected... but again, that's only for some parts anyway, and actually it'd be better if it did not compile those -- wastes a bit of time/compute).
Key point 2: As I write this, we support only (modern-ish) Linux, and X=true is supported (there is no good reason not to set X=true, ever, while we only support [modern-ish] Linux). (In #101 we of course want to go beyond Linux: macOs/ARM64 first, possibly later Windows.)
Key point 3: Currently X is a constant, and it controls certain if constexprs; end-of. So, if I wanted to write a unit_test with X=false, I couldn't: X=true is hard-coded.
Corollary: That's not fatal or anything. If N_s_s were a class template - not class - parameterized on X (or all of N_s_s_cfg that contains it; a-la config structs controlling boost.container configs + proposed for ipc::session in #146), then an X=false N_s_s could be instantiated and tested. (Perhaps rename N_s_s to Basic_n_s_s<...>; make backward-compatible using N_s_s = Basic_n_s_s<...with X=true...> in Linux.)
So: Problem: Here X=false is a future-proofing code path (and/or a "safety" code path, in case -- I guess -- someone doesn't want to use SEQPACKET and/or recvvmsg()). The only way to even build it would be to modify X's hard-coded value. Indeed, I have been keeping up with X=false -- but I can only locally test it; set X=false, run much of the pipeline locally (or do a personal branch + run real GitHub pipeline), do not merge into main (or check-in at all possibly). This isn't a good way of regression-testing. Solution Two parts basically: 1, parameterize N_s_s as sketched above (code change). 2, think of a way to at least lightly test the non-default config through pipeline. (Lots of approaches to this having achieved 1. Could add that stuff to the matrix full-on -- but that'll increase its runtime hugely, considering X is not the only variable, only one example -- or (as, say, with C++20 mode currently) add it to only certain compiler/whatever combos. Really, though, the difference between doing nothing and doing just one extra job with a different X setting = huge; adding other ones beyond, not so huge (utility-wise).
Well, details, TBD. You get the point.
--
However this ticket is not just about X. There are its 2 components, possibly independently, yes. There are others. Check N_s_s_cfg, which contains more things. Check all the code for others like it. Look for if constexpr first; and just do a qualitative survey. There won't be that many knobs/paths. Fewer than ~10.
Priority: It's not a matter of real code -- non-dead code -- going untested. It's more of a matter of avoiding buildup of technical debt/rot. As the (someday valuable) non-modern-Linux-default code paths keep living and being maintained, testing thereof relies on the maintainer's manually ensuring they're not left behind. So once the code does become non-dead (probably with #101), it'll be much less painful to proceed, as one won't have to figure out in what ways these code paths (such as X=false) have rotted.
So it's not high-priority in one sense, but in terms of tech-debt, the more time passes without this, the worse it gets. As I write this, I am actually pretty positive about how well these paths work and have been (manually) checked from time to time. But as time passes... you get the point.
--
There is also this big one, though it also applies to Flow and is a bit different/kinda overlapping. (TODO: File ticket for it, from the kernel of info in this description.) We test Flow and Flow-IPC against boost::thread threads (except that some recent tests do intentionally do otherwise, because they can without needing non-test code changes), but certain aspects (Flow's thread utils, Flow-IPC's advanced-thread-mechanics-using aspects including SHM-jemalloc especially) would benefit massively from also being equally tested against std::thread threads. (End-of-thread order-of-ops differing between the two is one direct source of risk. There could be more.) This presents a couple challenges:
- The same one above ["2"], which is just the question of how to add both kinds of runs to the matrix. (In this case it might be prudent to fully double the current matrix: once with std::thread threads, once with boost::thread threads. Then again, lots of tests would find this wholly irrelevant, and we should not waste time/compute.)
- Flow's flow.async module (esp Single_thread_task_loop but also its peers, all based on internal Task_qing_thread being the main fact here) is used by
all ( meaning there could be a few exceptions, probably very-very few) non-test code to start/manage threads. It uses boost::thread. But, in prod/non-test code that's in fact fine -- we can use whatever we want, why not. But-but, tests on the other hand -- and they too use flow.async for new threads, but also sometimes direct boost::threads (and, again, there might be a couple of tests that intentionally use both) -- should run both ways.
- Add configs for both, when no flow.async is involved.
- When flow.async is involved, nothing we can do... until we parameterize flow.async classes. E.g., Basic_single_thread_task_loop,
using S_t_tl = B_s_t_t_l<...boost threads...>, etc. (same basic idea as what was sketched for X above). So do that; then add configs for both.
On the test front, for this thread-provider dichotomy, I doubt it's necessary to extend the matrix; just go through the tests and add repeats of existing tests (that would benefit from both configs), one for each config (boost vs std). Some busy-work and investigation, but it's not that hard. We have large amounts of such things for, e.g., jemalloc vs classic / MQs vs sockets vs both / etc. So just another form of that.
Repeat: This doesn't quite match the main topic of the ticket; it is related; file separate ticket per TODO above.
I explain this by example (then I'll speak more generally). Native_socket_stream_cfg:: has two key
constexprflags; for our purposes here, just for discussion, we can consider these one flag X; just assume for now both component flags are either true or false.X = S_USE_OS_DGRAM_SUPPORT = S_USE_OS_DGRAM_BATCH_SUPPORT
If X=true, then Native_socket_stream will use Unix domain sockets in SOCK_SEQPACKET mode, and it'll use receive-batching via recvmmsg() (note two
ms) if asked to batch-receive; furthermore struc::Channel will auto-use a batching code path.If X=false, then N_s_s will use the classic SOCK_STREAM mode instead, which is in our case slower and trickier; and receive-batching just internally uses "emulation" (simply combines 1-message receives); furthermore struc::Channel will revert to a non-batching code path.
Key point 1: This dichotomy is compile-time. The path-not-taken is not even compiled in most cases; and though a couple nooks and crannies are compiled due to certain C++ technicalities, it is never called by anything (possibly optimized away in the end... but at least a compile error would be detected... but again, that's only for some parts anyway, and actually it'd be better if it did not compile those -- wastes a bit of time/compute).
Key point 2: As I write this, we support only (modern-ish) Linux, and X=true is supported (there is no good reason not to set X=true, ever, while we only support [modern-ish] Linux). (In #101 we of course want to go beyond Linux: macOs/ARM64 first, possibly later Windows.)
Key point 3: Currently X is a constant, and it controls certain
if constexprs; end-of. So, if I wanted to write a unit_test with X=false, I couldn't: X=true is hard-coded.Corollary: That's not fatal or anything. If N_s_s were a class template - not class - parameterized on X (or all of N_s_s_cfg that contains it; a-la config
structs controlling boost.container configs + proposed for ipc::session in #146), then an X=false N_s_s could be instantiated and tested. (Perhaps rename N_s_s to Basic_n_s_s<...>; make backward-compatibleusing N_s_s = Basic_n_s_s<...with X=true...>in Linux.)So: Problem: Here X=false is a future-proofing code path (and/or a "safety" code path, in case -- I guess -- someone doesn't want to use SEQPACKET and/or recvvmsg()). The only way to even build it would be to modify X's hard-coded value. Indeed, I have been keeping up with X=false -- but I can only locally test it; set X=false, run much of the pipeline locally (or do a personal branch + run real GitHub pipeline), do not merge into
main(or check-in at all possibly). This isn't a good way of regression-testing. Solution Two parts basically: 1, parameterize N_s_s as sketched above (code change). 2, think of a way to at least lightly test the non-default config through pipeline. (Lots of approaches to this having achieved 1. Could add that stuff to the matrix full-on -- but that'll increase its runtime hugely, considering X is not the only variable, only one example -- or (as, say, with C++20 mode currently) add it to only certain compiler/whatever combos. Really, though, the difference between doing nothing and doing just one extra job with a different X setting = huge; adding other ones beyond, not so huge (utility-wise).Well, details, TBD. You get the point.
--
However this ticket is not just about X. There are its 2 components, possibly independently, yes. There are others. Check N_s_s_cfg, which contains more things. Check all the code for others like it. Look for
if constexprfirst; and just do a qualitative survey. There won't be that many knobs/paths. Fewer than ~10.Priority: It's not a matter of real code -- non-dead code -- going untested. It's more of a matter of avoiding buildup of technical debt/rot. As the (someday valuable) non-modern-Linux-default code paths keep living and being maintained, testing thereof relies on the maintainer's manually ensuring they're not left behind. So once the code does become non-dead (probably with #101), it'll be much less painful to proceed, as one won't have to figure out in what ways these code paths (such as X=false) have rotted.
So it's not high-priority in one sense, but in terms of tech-debt, the more time passes without this, the worse it gets. As I write this, I am actually pretty positive about how well these paths work and have been (manually) checked from time to time. But as time passes... you get the point.
--
There is also this big one, though it also applies to Flow and is a bit different/kinda overlapping. (TODO: File ticket for it, from the kernel of info in this description.) We test Flow and Flow-IPC against boost::thread threads (except that some recent tests do intentionally do otherwise, because they can without needing non-test code changes), but certain aspects (Flow's thread utils, Flow-IPC's advanced-thread-mechanics-using aspects including SHM-jemalloc especially) would benefit massively from also being equally tested against std::thread threads. (End-of-thread order-of-ops differing between the two is one direct source of risk. There could be more.) This presents a couple challenges:
all (meaning there could be a few exceptions, probably very-very few) non-test code to start/manage threads. It uses boost::thread. But, in prod/non-test code that's in fact fine -- we can use whatever we want, why not. But-but, tests on the other hand -- and they too use flow.async for new threads, but also sometimes directboost::threads (and, again, there might be a couple of tests that intentionally use both) -- should run both ways.using S_t_tl = B_s_t_t_l<...boost threads...>, etc. (same basic idea as what was sketched for X above). So do that; then add configs for both.On the test front, for this thread-provider dichotomy, I doubt it's necessary to extend the matrix; just go through the tests and add repeats of existing tests (that would benefit from both configs), one for each config (boost vs std). Some busy-work and investigation, but it's not that hard. We have large amounts of such things for, e.g., jemalloc vs classic / MQs vs sockets vs both / etc. So just another form of that.
Repeat: This doesn't quite match the main topic of the ticket; it is related; file separate ticket per TODO above.