cppalliance-bot
commented
11 months ago An automated preview of the documentation is available at https://217.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html
Closed joaquintides closed 11 months ago
cppalliance-bot
commented
11 months ago An automated preview of the documentation is available at https://217.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html
pdimov
commented
11 months ago It might be useful for user to know whether unordered_x<...>::bulk_visit_size depends on the template parameters of the container, but I'm not quite sure how to express that.
joaquintides
commented
11 months ago Well, the fact that bulk_visit_size is a nested value of concurrent_x<...> implies that it depends on .... If we want to communicate that this is not the case, we just define it as a lobal constant ourside concurrent_x.
pdimov
commented
11 months ago Does it have to depend on them, though? Or on all of them? How would a different hash function possibly affect it?
joaquintides
commented
11 months ago Well, if we were to fine tune bulk_visit_size, it'd depend on Hash (the faster Hash, the larger bulk_visit_size ought to be) and on the CPU architecture (memory fetching time/instruction throughput). No other dependencies, really. The hot code is this:
Maybe all these considerations are moot, anyway, as we can always increase bulk_visit_size wholesale is there's a need: there should be no practical downside to having this constant larger than the minimum required to achieve no waiting cycles --more data stored on the stack, that's all.
codecov[bot]
commented
11 months ago Merging #217 (5afa714) into develop (44b669a) will decrease coverage by
0.03%. Report is 1 commits behind head on develop. The diff coverage is98.41%.
Some points for discussion:
visit(first, last, f)API wherestd::distance(first, last)is not known at compile time: benchmarks indicate there's no significant performance penalty vs. a fixed-size API."visit"rather than say"bulk_visit"for symmetry with other iterator operations likeinsert(first, last).std::forward_iteratorin C++20 and later: these two checks are not exactly equivalent.Hash,Predare not transparent,*firstmust dereference exactly to a (const) (reference to)key_type, instead of allowing for implicit conversions tokey_type. This is done out of conservatism cause*firstcan be called twice or more times for each key, and the conversion may be expensive.bulk_visit_sizeis a constant member value of the container rather than a global constant, even though it's the same everywhere (16). I feel this allows for more flexibility looking forward, but the ergonomics are a little worse for users implementing their buffering utilities.gtl::parallel_flat_hash_mapin the benchmarks makes the performance ofboost::concurrent_flat_mapin bulk mode degrade (!!). This does not make sense at all, but such is life. If you have a clue please let me know.