|
Flow-IPC 3.0.0
Flow-IPC project: Full implementation reference.
|
boost::asio::ip::tcp::socket. Similarly, async-ops could be made available as C++26 std::execution senders. (The sync_io cores would make either adapter fairly natural to build.) boost::has_trivial_destructor_after_move<ipc::shm::stl::Stateless_allocator> specialization: remove that specialization. As of Boost-1.92 the bug is there. The to-do preceding this one in the source code has corollaries: There are probably some (not all) _imply classes/class templates that are in detail/ but should not be, given that formally ipc::session is user-extensible. Again, though: 1, detail/ or not it (for each "it") can still be accessed in a pinch; 2, arguably more saliently, extending ipc::session hierarchies is a rare thing for a user to want to do in practice. It's an API-versus-SPI situation that isn't fully clean here. (Generally in Flow-IPC we endeavour to keep that fully clean.)
Session_base is in detail/ and is formally documented at the top of its doc header as "internal"; and indeed it is out-of-the-box; but the ipc::session hierarchies are also formally user-extensible, so Session_base should be publicly exposed after all. Hence remove the "internal" verbiage; move the file; and go over its source code/design, in case something about its API is too "Inside Baseball." To be clear, though, this is not of the highest priority: 1, detail/ or not it can still be sub-classed in a pinch; 2, arguably more saliently, extending ipc::session hierarchies is a rare thing for a user to want to do in practice. It's an API-versus-SPI situation that isn't fully clean here. (Generally in Flow-IPC we endeavour to keep that fully clean.)
Session dtor await (and possibly block) the start of the opposing side's Session dtor, before it can proceed. The reason it does this today is above this to-do in the code. The blocking is not too bad – if the user's applications are properly written, it will not occur, and the existing logging should make clear why it happens, if it happens; but it's still not ideal. There are a couple known mitigating approaches, and at least 1 ticket covering them is likely filed. (Either way the Graceful_finisher mechanism could then be removed probably.) To summarize them: 1, there can be a mechanism deep inside SHM-jemalloc code that gives (arbitrarily long, configurable) grace period for known borrowed objects whose session channels have closed before those objects were returned; this was echan's idea on this topic. Once the grace period is reached, it would act as-if they got returned then (so in the above scenario the arena could get jemalloc-deinitialized and all). If program needs to exit, it could either block until the end of the grace period or... who knows? 2, something similar could be done about the SHM-internal-use channel: do not close it but add it to some list of such channels; they would be finally closed upon detection of the other side's Session dtor being reached, in some background thread somewhere. (On 2nd thought this sounds a lot like Graceful_finisher – but for this particular internal-use channel only. The code here would only get more complex, though maybe not too much more. However, it would resolve the observed Session-dtor-blocking user-visible issue, at least until the delayed-channel-death process had to exit entirely.) Beyond these basic ideas (which could even perhaps be combined) this requires some thinking; it is an interesting problem. In the meantime the dtor-cross-process-barrier-of-sorts existing solution is pretty decent. Session / Session_server template parameters into a struct; possibly per-Client_app knobs). Its constants may then become run-time and/or per-session-configurable. Server_session::open_channel() and Client_session::open_channel() both emit error::Code::S_SESSION_OPEN_CHANNEL_SERVER_CANNOT_PROCEED_RESOURCE_UNAVAILABLE when unable to acquire the resources (as of this writing likely MQ-related ones) underpinning the would-be channel; but server-side the actual original Error_code could be emitted instead. (Client-side, due to the IPC-mechanics involved, this is less straightforward, as then: (1) it would be ambiguous as to whether it was the server-side-user-yanked-channel-immediately scenario (user's choice, not an environmental problem), or the current-RESOURCE_UNAVAILABLE scenario; and (2) transmitting an Error_code over IPC is a potentially subtle impl task. Hence the to-do is probably aimed at Server_session::open_channel() et al, not the client-side method family.) sync_io-pattern core is adapted by an async-I/O-pattern wrapper (as done in the perf-conscious ipc::transport unstructured-IPC impls like Native_socket_stream) would likely be much simpler and more elegant. Most likely this would need to be extended to Server_session hierarchy (heavily interacts with _server_impl). (Client_session hierarchy would at that point pretty much have to follow at least for consistency.) This is a major internal refactor – should not be taken lightly – and incidentally would likely precede any attempt at the to-do elsewhere, where we endeavour to make ipc::session user-extensible. (A bit of historical background: The internal impl of async-I/O-pattern hierachies in ipc::session preceded the very existence of the sync_io-pattern as an idea. Hence writing sync_io-pattern adapters around the already-written async-I/O core – e.g., Session_server_adapter – was more expedient than doing the reverse. With the benefit of extra hindsight, though, we feel more strongly that (while the status quo is acceptable in and of itself), subsequent maintenance would benefit from the refactor.) As a bonus/corollary there would be many fewer threads about per session and per server (not a real perf benefit but pretty good). virtual hierarchies Session_server, Server_session, Session_server_impl, Server_session_impl cooperate internally, as there is some funky stuff going on, particularly Session_server_impl::this_session_srv(). stats_forget_by_arena() which would lock, clear Borrower_shm_pool_collection_repository::m_per_arena_borrower_pool_stats, unlock; this would be available for tests that would like to start with/leave a clean slate for this set of stat; possibly even available via forward (like stats_reset()) by a public-facing API like Shm_session. The impl is simple and correct (won't create imbalanced stats or anything like that). For tests it may be quite helpful (as of this writing has not been needed though). Would tentatively avoid making a public-facing forwarder, though, as it feels fussy; the whole mechanism should already just work, when it comes to production use. Error_code (e.g., Flow-style via Error_code* out-arg), then Code::S_SHM_ARENA_CREATION_FAILED should go away. Error_code instead of reporting just success-versus-failure and reporting problem through an async callback, then ipc::session::shm::arena_lend::jemalloc::error::Code::S_SHM_ARENA_LEND_FAILED should go away. See Session_impl::init_shm() body for more discussion. Client_session is an alias to Session_mv parameterized a certain way, so the alias is defined inside Session_mv and is written in terms of Client_session_adapter due to knowing this fact. Maybe jemalloc::Client_session should be a thin wrapper instead of an alias, but that's a ton of lines for such a small thing... or maybe some rebind craziness would work.... shm::arena_lend::Shm_session, handling sessions dealing with arena-lending SHM-arenas backed by any memory-manager-backed Arena type (which could become a template parameter). Historically that had likely been a tall order, but looking at Shm_session insides now, it is actually (1) fairly contained in its duties, and (2) seems to make no jemalloc-specific mentions other Arena itself (which, again, would become a template parameter). (Regarding "fairly contained
in its duties": E.g., borrowed-pool and borrowed-object handling is now in Thread_lcl_obj_db_* and Borrower_shm_pool_collection_repository singletons – both already Shm_arena-parameterized and -segregated. Owner_shm_pool_repository handles owner-side pools and objects. Shm_session forwards things to them when needed is all.) flow::async::Task_asio_err for shm_channel_error_handler. There is zero functional difference, as internally it is stored as a Task_asio_err (which a Function<>) anyway; but for consistency and maintainability it'd be nice. Client_session is an alias to Session_mv parameterized a certain way, so the alias is defined inside Session_mv and is written in terms of Client_session_adapter due to knowing this fact. Maybe classic::Client_session should be a thin wrapper instead of an alias, but that's a ton of lines for such a small thing... or maybe some rebind craziness would work.... Difference_type template-param configurability. There are some rigid constraints, internal to Shm_pool_offset_ptr_data (core of Shm_pool_offset_ptr), on what it can be in practice. Moreover nothing, that we know of, needs this configurability. (offset_ptr has a wider range of use-cases, and no such rigid restrictions, so in its case this configurability makes sense; and/or at least it does not cause questions and worries in the same way.) The width of Lend_tracker_pool::Atomic_use_ct and the corresponding constant Use_count_registry::S_ALLOC_SZ is set to 4 bytes as of this writing; but it may be prudent to make this configurable at compile-time instead, to allow for the trade-off discussed in nearby "Tuning" doc header section.
In Lend_tracker_pool::Atomic_use_ct doc header, explain formally and practically how accesses to these values work from the point C++ abstract-machine PoV.
insert() vs register_shm_pool(). Though, the status quo is defensible; Borrower_... registers 2+ types of things; Owner_... only SHM-pools. Consider extending Shm_pool_offset_ptr impl, currently non-portably limited to x86-64 architecture (and gcc/clang compilers), to other modern architecture(s) (perhaps ARM and other mobile-related arch?).
Examine empirical performance impact of bit-field approach in Shm_pool_offset_ptr impl versus alternative bit-arithmetic approach (shift, AND, OR, NOT). I.e., benchmark it.
Examine portability details of bit-field approach in Shm_pool_offset_ptr impl.
Deleter_func than the existing choice Function<>. See idea(s) above this to-do. arena_id_t into detail/. public (accessible by said internals via a facade/attorney/etc. pattern of choice). A careful effort is required to clean up the API and then classify what's left as either public-facing or otherwise. Suggest also converting currently-used protected-for-unit-tests pattern to the facade/attorney/etc. pattern of choice (if necessary a separate facade/whatever could be used for internal-code consumers versus test-code consumers). E.g., start() is such a test-accessed protected-only-for-that-reason item. / Lastly: the public super-class Owner_shm_pool_collection, and its super-class(es), likely expose some public things that too should be gated-off from end-users. Ipc_arena::create() would be nice to emit a specific fitting error, as opposed to the somewhat ambiguous resource_unavailable_try_again-or-similar, when pool_name_base is malformed. The message could even suggest Shared_name::sanitize() and/or Shared_name::sanitized().
Ipc_arena::create() should follow normalized Flow error emission semantics (take Error_code*, set to success or specific failure, if null then throw on failure, encapsulating the truthy Error_code).
std::abort() if the assertion-trip attempt did not do it. flow::util::Thread_local_state_registry could compile-time-check for existence of a public such ctor and avoid compiling (optional) code that would use it – and in static-init of Static_state::m_caches we supplied a new Thread_cache optional-arg in that private context; or (less ideally) if we made a facade sub-class of Thread_cache that would expose this ctor but only to Thread_local_state_registry. For now the ctor is public but with that @warning and this to-do. owner_id_t into detail/. atomic<size_t> m_n_pools and m_mapped_sz; (2) implement their tracking by live-updating them via fetch_add() and fetch_sub(), whenever the existing m_pool_create/destroy_* values are updated; (3) add _hi_wmark counterparts to both; and (4) update them in the usual way; these will (as is typical elsewhere in Flow-IPC) become roughly the reason to even keep separate GAUGEs which are otherwise derivable from existing create/destroy ACCUMULATORs. Until all that, the existing ACCUMULATORs are sufficient; and the _destroy_ stat-members are nice canaries to detect unexpected (non-shutdown) pool-removals. Avoid relying on boost.ipc internals for the impl of Sparse_managed_shm; reimplement Sparse_managed_shm::Region ourselves. The algorithm is somewhat hairy, but it's not that long, and we can skip some inapplicable complications/knobs, as we're targeting only POSIX-SHM specifically: not XSI, not files, etc. Suggest carrying this out before attempting the to-do above wherein we'd make Sparse_managed_shm user-facing.
bipc_ext::Sparse_managed_shm is currently in detail/ (formally not user-facing) and, as explained above this to-do in the code, provides access to a very similar API but is not identical; the to-do would be to (1) make it (close to) identical and (2) move it out of detail/. For details of (1) consult the nearby comparison of the two APIs. If (1) proves impractical – e.g., if core() proves difficult to eliminate – then (2) alone, with some quality-of-life tweaks such as adding move semantics, would still be quite nice. / As of this writing it hasn't been done for reasons of expediency only. This move would fit well with doing a similar to-do on Sparse_shm_object; an example of "synergy" is that the latter is used by Sparse_managed_shm. In terms of priority: basic_managed_shared_memory is a crown jewel of usefulness in bipc, and among power users the lack of sparse/on-demand RAM-commits in later Boosts can be surprising and unpleasant. So while Flow-IPC's overall mission does not center on bipc per se, this particular utility in and of itself may be quite useful indeed to a power user of bipc. I.e., it'd be an easy sizable win despite, at the moment, being something Flow-IPC uses only internally.
bipc::shared_memory_object; so making it publicly available as a utility/bipc extension would be a net win. As of this writing it hasn't been done for reasons of expediency only. This move would fit well with doing a similar to-do on Sparse_managed_shm; an example of "synergy" is that the latter uses Sparse_shm_object. (However that "similar to-do on `Sparse_managed_shm`" involves some light design and coding, while the present to-do is pretty much "move it out of detail/ and accordingly between `_fwd.hpp`s." Suggest doing both ~simultaneously. While Sparse_shm_object would be publicly "potentially fairly useful," we estimate Sparse_managed_shm would be significantly more so, as basic_managed_shared_memory is a crown jewel of usefulness in bipc, and among power users the lack of sparse/on-demand RAM-commits in later Boosts can be surprising and unpleasant.) m_[non_owner_]destroy_count stat-pair could be implemented: count how many times a borrowed-obj's disposer was the last disposer (for the underlying object) to execute, versus how many times it was the original constructed-obj's disposer. This would require storing an atomic count in SHM, almost certainly in arena_lend::detail::Lend_tracker_pool, similarly to Lend_tracker_pool::Metadata::m_n_unused (summed across threads at stat-consumption time). This has an impact on cross-thread perf and is not wholly trivial to code; so we should approach this only if there is demand in the field. All else being equal, though, it would be an interesting signal. Arena::Pointer shall be a fancy-pointer, but we could support raw pointers also. Suppose Arena is set up in such a way as to map all processes' locally-dereferenceable pointers to the same SHM location to the same numeric value (by specifying each pool's start as some predetermined numerical value in the huge 64-bit vaddr space) in all processes sharing that SHM pool. Now no address translation is needed, and Arena::Pointer could be simply T*. As of this writing some inner impl details assume it being a fancy-pointer, and it's the only active need we have; but that could be tweaked with a bit of effort. asio_local_stream_socket additional feature: APIs that can read and write native sockets together with accompanying binary blobs can be extended to handle an arbitrary number of native handles (per call) as opposed to only 0 or 1. The main difficulty here is designing a convenient and stylish, yet performant, API. Having achieved this we could also similarly extend the Native_handle_sender, Native_handle_receiver concepts and their impls.
Analogously to how Msg_batch_in supports batch-receipt, we could add API(s) for batch-send. Having achieved this we could also similarly extend the Blob_sender, Native_handle_sender concepts and their impls. Without getting into details: sending and receiving, especially in batches, are not really mirror images of each other conceptually. Hence lacking one but not the other is not necessarily an omission due to lack of time; the need for it is not as clear (we feel); and the API(s) would probably be designed fairly differently.
Native_handle is provided: Msg_batch_in::result_payload_hndl(); a receiver-engine rewinding n_used() (un-emitting slots) can thus close a stored handle (via the copy) but not nullify the stored value, leaving a stale (closed) handle value recorded in a meaningless-territory slot. For cleanliness/defensiveness an engine-facing API – e.g., close-and-nullify over an index range – might be helpful or required; see its would-be use in Native_socket_stream_msg_batch_in::nb_read() rewind logic. Suggest also applying identical changes to Generic_msg_batch_in API, async_receive_batch_emulation() doc header documenting the tparam Batch, and accordingly the rewind logic inside async_receive_batch_emulation_on_init_msg(). Both rewind spots as of this writing are marked with associated to-dos. enum with the codes, like error::Code; (2) an int-to-string message table function, for log messages; and (3) a brief const char* identifying the code set, for log messages. The rest is the boiler-plate, but all of it seems to either already be acceptably brief, and where something isn't quite so, I can't think of any obvious way to factor it out. Of course a macro-based "meta-language" is always a possibility, as we did in flow::log, but in this case it doesn't seem worth it at all. Comparing Blob_sender::send_blob_max_size() (and similar checks in that family of concepts) to test whether the object is in PEER state is easy enough, but perhaps we can have a utility that would more expressively describe this check: in_peer_state() free function or something? It can still use the same technique internally.
In C++20, if/when we upgrade to that, Native_handle_sender (and other such doc-only classes) can become an actual concept formally implemented by class(es) that, today, implement it via the "honor system." Currently it is a class #ifdef-ed out from actual compilation but still participating in doc generation. Note that Doxygen (current version as of this writing: 1.9.3) claims to support doc generation from formal C++20 concepts.
Channel-opening capabilities – the user must come up with their own naming scheme that avoids name clashes; we could supply an ipc::session-facilitated system for providing this service instead. I.e., ipc::session could either expose a facility for generating the Shared_name absolute_name arg to the Native_socket_stream_acceptor ctor (and opposing Native_socket_stream::sync_connect() call). Alternatively it could provide some kind of Native_socket_stream_acceptor factory and corresponding opposing facility. Long story short, within the ipc::session way of life literally only one acceptor exists, and it is set up (and named) internally to ipc::session. We could provide a way to facilitate the creation of more acceptors if desired by helping to choose their Shared_names. (An original "paper" design did specify a naming scheme for this.) shared_ptr-based ones, possibly focusing on boost.asio socket objects in particular. false; otherwise it always represents a message-type only. We have kept the existing name arguably form legacy inertia and/or because a better name was not immediately forthcoming. unique_ptr. For example, imagine a Session::Structured_channel Session::structured_channel_upgrade(Owned_channel_t&& channel, ...) that constructs a suitably-typed struc::Channel, subsuming the raw channel just opened in that session::Session, and returns that guy. Currently it would need to return unique_ptr<Session::Structured_channel> or something. struc::Channel::sync_end_sending(), possibly with timeout, since async_end_sending() to completion is recommended (but not required) to execute at EOL of any struc::Channel. The "blocking" part is good because it's convenient to not have to worry about handling completion with async semantics boiler-plate. The "timeout" part is good because it's a best-effort operation, when in a bad/slow environment, and blocking – even during deinit – would be important to avoid. For example, the Session impls' dtors perform this op; we don't want those to block for any significant time if at all avoidable. The reason it's a mere to-do currently is that a bug-free opposing struc::Channel should not let would-block occur for any real length of time; so blocking is presumably unlikely. Nevertheless. m_segment_sz_init knob, but in practice (1) at least struc::Channel::create_msg() impl references it, as if it must exist and affect the inner-most serialization's seg1; and (2) the 2 out-of-the-box impls Heap_fixed_builder and shm::Builder provide it; so this requirement should be formalized in the concept definition. Most users will not roll their own Struct_builders, and those that do will presumably figure out the issue if their code fails to compile; but formal correctness is a good thing. struc::Channel::auto_ping() and struc::Channel::idle_timer_run() and, for safety, removing struc::Channel::owned_channel_mutable(). Improve ipc::transport::struc::Channel::sync_request(), so that 2 such calls on the same Channel can execute without one having to fully block the other. Note this may be seen as a breaking change of sorts for the purposes of release notes (even though it would presumably be seen as an improvement).
Consider treating a response to a timed-out struc::Channel::sync_request() as an unexpected response: firing the set_unexpected_response_handler() handler locally (perhaps with an added flag indicating expected-but-timed-out) and informing the opposing peer, so that its set_remote_unexpected_response_handler() handler fires. As of this writing such a late response is simply dropped.
Segment_ptrs element type would ideally be a view into Blob-or-similar instead of actual Basic_blob*, to decouple impl from API more cleanly. One approach might be to develop a flow::util class/template, a peer of the flow::util::Blob hierarchy, that specifically represents a view into a Blob-type-thing (perhaps 1 variant for mutable, 1 variant for immutable), perhaps analogously to how span<uint8_t> + span<const uint8_t> is each a view into vector<uint8_t> (minus the latter's capacity). It would therefore store a pointer, a size, a capacity, and a start-index – representing a main buffer (size() in size), N-byte header/prefix (N = start()), and M-byte postfix (M = capacity() - start() - size()). (A more economical buffer+header version – directly applicable here – might store all of those minus the capacity, meaning there is no postfix area.) Possible names might be Blob_view_const (read-only view into buffer plus header plus postfix), Blob_view_mutable (same plus writable access), Prefixed_byte_span_const (Blob_view_const minus postfix area), Prefixed_byte_span_mutable (Blob_view_mutable minus postfix area). Segment_ptrs would likely be vector<Prefixed_byte_span_mutable>. Blob. The existing one is suitable for the main use-case which is internally by shm::Builder; but Capnp_message_builder is also usable as a capnp::MessageBuilder directly. If a user were to indeed leverage it in that latter capacity, they may want to transmit/store the SHM-handle some other way. rebind in the impl of Capnp_message_reader::Borrower_allocator. rebind in the impl of Capnp_message_reader::Segment_in_shm_borrowed. rebind in the impl of Capnp_message_reader::Segments_in_shm_borrowed. sync_connect()/async_accept() invoker. Instead the channel is for this: Client sends one native handle (half of a socket-pair used for the capnp-RPC conversation by the Session_vat_network) over it. Having received it the server closes the channel. On the client side, here, Client_context retains its side of the channel indefinitely (reason: see sync_connect_impl() body; but basically because it does not know when opposing server has actually consumed the socket-pair). / Without Protocol_negotiator, which admittedly would complicate the impl annoyingly, needing to change this protocol in the future (such as to avoid keeping the aforementioned channel open indefinitely) is not a very civilized proposition. To deal with this: Just in case, for now, we've thrown in a reserved value along with the native handle. Alternatively one can always increment an ipc::session Protocol_negotiator version (both local_(min|max)_proto_ver <=> no compatibility with older protocol), if a change to this subsequent RPC-establishment protocol is required. After all both this module and ipc::session are parts of Flow-IPC. n_init_channels_by_srv_req_func arg, it would be a nice improvement to the contract to guarantee issuing the call to that user-function from the KJ-event-loop thread, like the rest of the async action. (E.g., the following async parts execute in the KJ style: promise fulfillment; transport_method_func() call if applicable.) This API change would not be a breaking change. The reason it is not already done is that the Flow-IPC capnp-RPC-support layer treats the non-RPC-aware ipc::session layer (normally used directly by user but not here) as a black box; and the session::Session_server::async_accept() contract – in this particular detail – did not need to be KJ-friendly (or even KJ-aware) in its design, so it was not. (capnp-RPC support did not exist at the time.) So the way session::Session_server invokes n_init_channels_by_srv_req_func() is inverted versus what the KJ flow needs. To implement this improvement, session::Session_server would need an added async hook for the give-me-the-desired-channel-count step, and Context_server::accept() impl would make use of it. In the ipc::transport::struc::Struct_builder-centered capnp-serialization module, a very perf-critical feature would be a serializer combining struc::Heap_fixed_builder and struc::shm::Builder, in that it would inline (in the for-IPC-transmission byte stream) smaller segment(s?) while SHM-storing larger ones (as shm::Builder does for all segments currently); integrate this work with both struc::Channel and capnp-RPC (rpc::Session_vat_network et al) structured-transport systems. shm::Builder and its MessageBuilder-implementing core shm::Capnp_message_builder would perhaps be extended to support this new – faster! – mode, with the ability to fallback to its current SHM-always behavior (and perhaps to Heap_fixed_builder's heap-always behavior if so desired). In designing this, there are two major questions to resolve first: 1, what is the fastest-behaving solution (where is the cut-over where SHM starts winning? etc.); 2, what is the best integration point between rpc::Session_vat_network and the new serializer. (Integration with struc::Channel is already through the Struct_builder + Struct_reader concepts, here implemented as shm::Builder and shm::Reader respectively. Session_vat_network's possible integration methods are more of an open question. The ticket (https://github.com/Flow-IPC/ipc/issues/160) may contain detailed ideas.)
rpc::Session_vat_network and probably some of the layers built on top of it should make capnp::ReaderOptions configurable – analogously to the vanilla capnp::TwoPartyVatNetwork. (Be sure to apply this judiciously to the correct layer of serialization; and document thoughtfully, as the practical perf implications here are different from the vanilla/non-zero-copy use case.)
optional, and it is fine with at least gcc, but clang gives an error; so for now made it unique_ptr; look into it sometime. It's not a huge deal as a unique_ptr though. Presumably it's something inside BufferedMessageStream triggering this; capnp-1.0.2; clang 13, 15, 16, 17; C++17 mode at least. The error is: "the parameter for this explicitly-defaulted copy constructor is const, but a member or base requires it
to be non-const" – referring to optional copy ctor. shared_ptr and/or weak_ptr or something? E.g., we did so in the case of Ez_rpc_kj_io (internal to Ez_rpc_* stuff). Just stylistically we prefer that sort of thing to kj::Own craziness. This would be pretty easy, and pithy at that, if we could return shared_ptr<Rpc_conn>, but the interface requires Own<Rpc_conn>. So maybe just leave it. 10 as a reasonable default... so is it reasonable? I (ygoldfel) just picked something that felt like it would probably be more than enough in most conceivable uses of capnp-RPC's native handle (a/k/a FD in Unix a/k/a capability in capnp-RPC parlances) passing feature: really a capnp-interface impl can only feature a single handle via getFd(), and one guesses that even with all the promise-pipelining ever, a single internal rpc::Message is unlikely to combine more than 10 of these... probably? On the other hand, why not just make this, like, 250-ish? Answer: Comments in capnp suggest this whole thing is a "security" thing, but I personally lack the background as to the concerns discussed there. The to-do is to reach and document a greater level certainty about this value (and change it if appropriate). N == 1 to something larger, so that the user can optionally – while still using this global stats-holder – further categorize stats according to their custom needs. Also consider similar steps in such alias(es) in struc::shm::stat. (Note various relevant APIs allow one to simply pass-in a Serializer_stats* (et al) directly – and the user can maintain their own Global_stats or static Stat_set_list and lightning-quickly obtain the aforementioned * therefrom. So flexibility already exists on that front; but here we use a Global_stats for a reason already; can make use of it more fully this way.) One tactical idea is that factories like struc::Channel::heap_fixed_builder_config() can take an optional arg, defaulting to 0, which would cause such a factory to select the Global_stats slot. Slot 0 would continue to, by convention be the default/catch-all one; and the user can designate other slots for their own purposes. any instead of shared_ptr<void> might be slightly prettier. As noted ahead of this to-do, though, the only places that would actually change at all would be at init (start_and_poll()) and ~at destruction. Perf-wise that means it doesn't matter. Style-wise it would only matter in start_and_poll() in ~3 lines and here at the declaration site. Arguably any (all else being equal, which it is here) is simply a more expressive way to represent a type-erased value. It's just slow (in certain contexts – but not ours). if constexpr() w/r/t Mq::S_HAS_NATIVE_HANDLE-based compile-time branching: it could save some RAM by eliminating optionals such as the one near this to-do; though code would likely become wordier. m_user_request->m_target_blob (instead of locally stored m_target_control_blob) to save RAM/a few cycles? Technically at least user should not care if some garbage is placed there. if constexpr() w/r/t Mq::S_HAS_NATIVE_HANDLE-based compile-time branching: it could save some RAM by eliminating optionals such as the one near this to-do; though code would likely become wordier. ..._receiver in terms of a custom sync_io::..._receiver would cause a build error, unless the writer of the latter hypothetical custom guy added a S_TRANSMIT_NATIVE_HANDLES member. In practice they'd probably know to do so upon seeing the error and the present code; but formally a relevant concept should specify it, so the custom-guy writer would've known to do so from the start. vectors, deques (including inside queues). Peer_socket m_peer_socket synchronous-read ops (read_some()) are actually documented in boost::asio to be thread-safe against concurrently invoked synchronous-write ops (write_some()), as are OS calls "::recvmsg()", "::sendmsg()"; therefore for possible perf bump consider never nullifying Native_socket_stream_impl::m_peer_socket; eliminating m_peer_socket_mutex; and letting each direction's logic discover any socket-error independently. (But, at the moment, m_peer_socket_mutex also covers mirror-guy m_ev_wait_hndl_peer_socket, so that must be worked out.) Native_handle, possibly null). Then where today we invoke m_rcv_resume_incomplete_msg_processing_func if not-.empty(), instead check whether the saved payload-to-replay is 1+ bytes. If so replay as follows: Provide some hooks into the nb-read-next-bytes code which will first read from the payload-to-replay in memory; and once that runs out do the regular socket reads. This has its own subtleties, surely, but conceptually it does seem cleaner. Or! We could even do something ridiculous like setting up a temporary fully-local socket-pair and load it up with the payload-to-replay; then sorta-transparently have our nb-read stuff read from that socket first until exhausted. Whatever we do, this time really make it clean and as pithy as possible. No point in messing with it, unless it's really as nice as it can be. Consider making Native_socket_stream_impl::Rcv_user_batch_request a template on typename Batch; removing Native_socket_stream_impl::m_rcv_user_request and m_rcv_user_batch_request; and passing them around Native_socket_stream_impl insides via lambda captures and template-parameterized args. The motivation is consistency and, arguably, simplicity; see discussion in the preceding paragraph relative to this to-do in the code. Note that (1) this would be essentially a stylistic change (perf impact negligible) and (2) we'd still need to mark whether a user-request is in progress, to continue guarding against user attempts to launch simultaneous receive requests. For (2) we'd probably replace the struct members m_rcv_user_request and m_rcv_user_batch_request with a single bool m_rcv_user_request_active.
For a fairly-minor (especially since this is inside the impl, not the API) bit of consistency with the vaguely comparable sync_io::Async_adapter_receiver, we should arguably rename Rcv_user_request => Rcv_user_request_one, Rcv_user_batch_request => Rcv_user_request_batch. The former was kept as-is basically because pre-batch-feature that is what it was called.
operator>> and operator<< being asymmetrical get one into trouble when using Shared_name with boost.program_options (or flow::cfg which is built on top of it)? Look into it. It may be necessary to make operator<< equal to that of ostream << string after all; though the added niceties of the current << semantics may still at least be available via some explicit accessor. Consider adding a Shared_name_view which would be to Shared_name as String_view is to string; and that could itself be constructed from a String_view (and therefore from const char* [+ length], etc.). The main work would arguably not be the creation of that class itself but rather (1) ensuring proper implicit conversion between the main class and the new one and (2) conversion of the many Flow-IPC APIs that take a const Shared_name& that would (hopefully in essentially non-user-code-breaking ways) instead take Shared_name_view.
It would be good to constexpr-ify Shared_name to the extent possible; it might even make a dent in perf in some contexts. Internally this is annoying until C++20, where std::string itself got constexpr-ified, as we use std::string internally; best left alone until then.
public). There are pros and cons; the basic pro being that Shared_name is meant to be a very thin wrapper around std::string, so it might make sense to allow for in-place modification without supplying some kind of reduced subset of string API. Suggest doing this to-do if a practical desire comes about. sync_io-pattern use with an old-school reactor-pattern event loop, using poll() and/or epoll_*(). define TEMPLATE_ + define CLASS_ technique where it'd be subjectively beneficial. 1.9.4