Re: [review] The review of boost::container::hub starts! April 16 - April 26
Joaquin M López Muñoz via Boost <[email protected]>
| Newsgroups | gmane.comp.lib.boost.devel |
|---|---|
| Message-ID | <[email protected]> |
El 18/04/2026 a las 13:13, Amlal El Mahrouss via Boost escribió:
> Hello,
> I hope everyone is doing well.
>
> My review of `hub` will cover: Documentation, Design and the implementation of the hub container.
>
> ## DESIGN:
>
> - The presence of pre-conditions, a single container, and free functions is a big plus in the design, this lets the developer use the library without too much hurdle on the development side.
> - Speaking of development the presence of a NATVIS file is also a big plus in terms of debugging for Visual Studio. (IIRC a lot of users of Boost develop on Windows)
> - There is a clear cause and effect in the design as well, easy to reason about and solve problems upon.
> - The usage of known requirements like CopyInsertable in the design is also a big plus to avoid technical debt.
>
> ## DOCUMENTATION:
>
> - The format of the documentation consists of the function signature, preconditons, snyposis, and effects. It is another plus for users wanting to get started using the library.
> - The addition of equivalent examples is also a plus for the library, knowing what else can be done is as important as the solution.
>
> ## IMPLEMENTATION:
>
> - It has clean free functions like erase_if and is easy to read, which fits what the library `container` library should do.
>
> ```cpp
> template<typename T, typename Allocator, typename Predicate>
> typename hub<T, Allocator>::size_type
> erase_if(hub<T, Allocator>& x, Predicate pred)
> {
> using hub_container = hub<T, Allocator>;
> using size_type = typename hub_container::size_type;
> using block = typename hub_container::block;
>
> auto s = x.size_;
> for(auto pbb = x.blist.next; pbb != x.blist.header(); ) {
> auto pb = x.static_cast_block_pointer(pbb);
> pbb = pb->next;
> BOOST_CONTAINER_HUB_PREFETCH_BLOCK(pbb, block);
> auto mask = pb->mask;
> do {
> auto n = hub_detail::unchecked_countr_zero(mask);
> if(pred(pb->data()[n])) x.erase_impl(pb, n);
> mask &= mask - 1;
> } while(mask);
> }
> return (size_type)(s - x.size_);
> }
> ```
> Fig 1: erase_if implementation (https://github.com/joaquintides/hub/blob/develop/include/boost/container/hub.hpp)
>
> - The design pattern is respected per design guidelines and there is a separation of concerns.
> - There is a clear consideration for MSVC and other compilers, which is a big plus for adoption.
>
> ## EFFORT:
>
> I went and looked at the documentation, implementation and overall structure of the repository.
>
> ## COMMENTS:
>
> - The transparency about `BOOST_CONTAINER_HUB_PREFETCH` is really valuable, it's wroth finding more cases like this in the future and documenting them.
>
> ## CHOICE:
>
> I vote to ACCEPT `hub` to be merged into `container`. It clearly has a lot of upsides (including HFT, per the author's examples)
Hi Amlal,
Thanks for your review!
Best,
Joaquín M López Muñoz
_______________________________________________
Boost mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://lists.boost.org/mailman3/lists/boost.lists.boost.org/
Archived at: https://lists.boost.org/archives/list/[email protected]/message/RVOY3GO2Z25BWZXFNFU4ZK62IPFCM3SA/