champ for_each_chunk_p

[fabianbs96]

Currently, the immer set, map, and table containers only support a subset of available algorithms; especially immer::all_of is not supported. That is, because the underlying champ does not implement for_each_chunk_p.

This PR adds an implementation of for_each_chunk_p to the above mentioned champ. Should be related to #171.

Design considerations:

• iterative- vs recursive implementation: As `for_each_chunk_p may be part of extremely hot parts of user-code, I prefer to avoid non-tail recursion
• Worklist vs explicit stack: The easiest non-recursive implementation would be based on a worklist of const node_t *. However, this has the drawback of potential memory allocation, so the implementation uses an explicit stack (std::array) that models the required parts of the call-stack from for_each_chunk_traversal.
• Question: Should we std::invoke the callback if compiled with C++17 or higher?

[codecov-commenter]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (5875f77) 90.53% compared to head (94dc1fc) 90.54%.

❗ Current head 94dc1fc differs from pull request most recent head 6cd57a2. Consider uploading reports for the commit 6cd57a2 to get more accurate results

Files	Patch %	Lines
immer/detail/hamts/champ.hpp	86.66%	4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
+ Coverage   90.53%   90.54%   +0.01%     
==========================================
  Files         119      119              
  Lines       12144    12203      +59     
==========================================
+ Hits        10994    11049      +55     
- Misses       1150     1154       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arximboldi]

This is a great contribution thank you! Once gain, sorry for the delays in reviewing this...

...one of the reasons it took me long to review it, is I wanted to book time to understand the implementation properly, as a recursive implementation would have been easier to understand.

Have you tried to benchmark this and see if there is an actual performance benefit and, if so how much? It feels to me that you're doing more or less the same work than the compiler generates when using recursion normally (just a bit less, not saving the pointer to the code position). But I ask out of genuine curiosity as I really can't predict here what performance would be like, as modern compilers and processors are sometimes surprising when it comes to micro-optimizing...

[fabianbs96]

Hi @arximboldi,
when I implemented this, I also did some benchmarking, but I don't remeber the numbers anymore. You are right, modern compilers and processors already do a lot of optimizations, but in my experience (since I work in the area of static code analysis based on LLVM) compilers are especially bad, when it comes to recursion, so I try to avoid it whenver I can.
For example, the recursive calls are usually not inlined, although in this particular situation, inlining would make a lot of sense.

I might do some benchmarks to show some numbers, but I currently cannot tell you when I will find the time to do this.

[arximboldi]

My argument in this case is not that the compiler would inline the code, but that alternative code does something very similar to what the compiler does when making a normal function call (i.e. pushing the local vars in the stack), so you're kind of doing manually the compilers job for no potential performance gain. I may be wrong of course as this kind of micro-optimization is very nuanced.