Hi to all, I'm glad to announce that the proposed boost::container::hub container has been ACCEPTED. Congrats Joaquín! I'll try to summarize the comments/actions/changes/suggestions from reviewers. We have received 6 reviews: - Andrzej Krzemienski: Accept — "I recommend accepting Hub into Boost." - Amlal El Mahrouss: ACCEPT — merge into container. - Arnaud Becheler: ACCEPT — "I suggest: ACCEPT." - Christian Mazakas: ACCEPT — "strongly Accept" into Container. - Matt Borland: ACCEPT — explicit closing vote. - Emil Dotchevski: ACCEPT — "My vote: ACCEPT" (with a list of issues). - Peter Turcan: (documentation comments; no explicit ballot line) ------------------- Andrzej Krzemienski ------------------- Praises: - Easy to read and understand - Well documented, where needed. Main comments: - Recommends making the `visit` functions free functions and renaming them to `for_each` and `for_each_while` --> Joaquín agrees - std::hive and hub are not SequenceContainers, docs should make this explicit --> Joaquín agrees. - Questions BOOST_LIKELY --> Joaquín clarified BOOST_LIKELY is not the core performance story. - "memory()" is marked with "// TODO: remove;" --> Joaquín agrees to remove it. - Reported a GCC -Wall warning when sorting an empty hub (Peter Dimov flagged it as a false positive and suggested a fix) --> Joaquín agrees to fix this. ----------------- Amlal El Mahrouss ----------------- Amlal praises several aspects of the library: - Preconditions - Usage of known requirements (like CopyInsertable) - Free functions - NATVIS for MSVC - Implementation readability - BOOST_CONTAINER_HUB_PREFETCH transparency ------------ Matt Borland ------------ Points to: - Strong design decisions and clear deviations from std::hive - Usefulness is clear given std::hive in C++26; on testing - Clean implementation - Appreciates pretty printers Suggestions: - Stricter warnings (-Wall, -Werror, -Wold-style-cast) --> Joaquín will tighten the warnings - Questions MSVC minimum (14.3) vs very old GCC/Clang --> Joaquín will try to handle that with the help of Boost.CI - Finds ASCII performance tables hard to read, suggests implementing Peter Turcan-style improvements. - Reports 4 of 5 tests failed on ARM Mac with Clang 22 / GCC 15 --> PRs reported, one liner fix --> Joaquín will fix it and will improve the regression with the help of the latest Boost.CI improvements. --------------- Arnaud Becheler --------------- Praises: - C++11 portability - Benchmarks - Code looks clear - Docs Comments about docs that Joaquín will address: - trim_capacity example needs clarity - "Pointers and iterators to an element" section needs improvement - motivation could use a clearer competitor comparison - prefers comparative tables; ASCII tables are awkward (Joaquín clarifies final docs will not stay Markdown) - get_iterator needs a motivating story for users who only have T* (Joaquín admits uncertainty on real-world value of get_iterator beyond standard API parity) - Typos and line-break nits ----------------- Christian Mazakas ----------------- - General agreeement. - Minor caution that insertion order is not preserved when slots are reused, should be stated in the docs Christian also suggets a future extension for slotmap-style alternative. Joaquín agrees to review this suggestion in the future and also points to a proposal for a "reserving" (virtual memory) allocator idea as future exploration. ----------------- Emil Dotchevski ----------------- Likes: - The bitmask approach vs hive. - Very high quality implementation Notes/suggestions: - We should leave block size unspecified in documentation and removing explicit "64" in docs. - Possible leaks in constructors if block_list lacks cleanup (Joaquín clarified it's not the case) - sort_iterator::operator[] and operator* --> Joaquín will fix it - transfer_sort buffer and over-alignment --> over-aligned new[] will be used where available. - exception guarantee text for transfer_sort should say basic guarantee - get_iterator should assert validity Joaquín also said that he will add exception-safety tests. ------------ Peter Turcan ------------ Peter focused on clarity and adoption improvements: - Opening sentence and HFT acronym (should be clarified) --> Joaquín will revise intro - Deeper, less conversational use-case treatment - Explain the role of 64 and why it is not freely tunable --> Joaquín agrees (also comments 128 would be nicer if hardware had a single instruction) - Prefer plain-English comments - Clarify trim_capacity / reserve comments --> OK - Avoid excessive brackets on the performance section and clarify some numbers there --> OK - Reference could use more exception / example cross-links --> OK ------------ ------------ Joaquín is already working on changes based on the review feedback here: https://github.com/joaquintides/hub/tree/feature/review-feedback I will work with Joaquín to find the best way to integrate code/docs/tests into Boost.Container. Best, Ion