Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flow: ~All Flow public classes (inc. noncopyable) should be move-ctible and move-assignable. #75

Open
ygoldfeld opened this issue Mar 10, 2024 · 0 comments
Labels
enhancement New feature or request from-akamai-pre-open Issue origin is Akamai, before opening source

Comments

@ygoldfeld
Copy link
Contributor

Filed by @ygoldfeld pre-open-source:

~All Flow public classes (inc. noncopyable) should be move-ctible and move-assignable.

I've added it to a few opportunistically/on-demand, but we should do it comprehensively.

---detour---
E.g, Single_thread_task_loop and all other _loops should have it. It is sometimes annoying to make user classes movable, because they have _loops, etc., directly inside.

(EDIT: This particular thing -- move()ing an object that contains a _loop -- should contemplate specifically, as due diligence. What I mean is -- well, consider boost::asio::io_context. It is movable; but just because it is movable does not mean a containing object is easy to make move()able: an io_context is attached to things like, e.g., socket objects and outstanding completion handlers. It might require some surgery to move the whole set of objects, io_context included, correctly. I've resorted to pImpl or pImpl-lite (in case of templates) to add movability of such complex objects containing async state machines. So my point is, think about that when making the _loops movable: How well will it work when done?)
---end of detour---

It also, for me personally, has caused unnecessary over-use of mandatory-shared-ownership pattern in the past. That's fine but arguably a bit slow; and goes against the STL/Boost style of design. (EDIT: E.g., in Flow-IPC, we've almost entirely shied away from that: things are movable rather than mandatorily-shared-via-shared_ptr to achieve performantly passing around heavy objects. That is the direction in which we should head, but Flow originated in the days before this epiphany for me personally.)

They should (could) also have ADL-specialized swap(), when this would reasonably improve perf on top of the default move-ctor/move-assignment-based impl; in that case often the move-assignment can be written in terms of swap(); and move-ctor can be written in terms of move-assignment. E.g., util::Basic_blob does this. So do util::Linked_hash_map/set.

@ygoldfeld ygoldfeld added enhancement New feature or request from-akamai-pre-open Issue origin is Akamai, before opening source labels Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request from-akamai-pre-open Issue origin is Akamai, before opening source
Projects
None yet
Development

No branches or pull requests

1 participant