Unnecessary huge stack requirement by TurboPFor

[pps83]

You know yourself that to run it at all you have to increase default stack size to some massive number (as it overflows stack with default 1MB size). IMO, this is most likely a compiler bug (that seem to exist in all compilers imo). TurboPFor has huge switch with blocks of code, and compiler calculates stack use for each block and adds them all together as if separate switch blocks could also execute by the same function instead of merging these stack blocks. This results in that insane stack requirement. IMO, most projects might even skip on TurboPFor after seeing it stackoverflow without even trying to figure it out what happened.
Anyways, TurboPFor doesn't need this insane massive stack to operate with some code restructuring. I fixed it in my branch, and I'm not sure if that's something would be OK to take, as the change is massive and not trivial at all (uses some preprocessor magic to make it work, I actually wrote most of the repetitive code using editor macro).

This is the change that changed it from 1+MB stack to 40KB:
9c116bc

basically it makes all the switch blocks into standalone forceinline functions. When merging forceinline functions compiler doesn't not add stack that each of the function could use, instead it uses max size of them all which results in this reduction from 1+MB to 40KB

Also, lots of bitunpack functions should be static: fd60325

I have lots of other improvements and real bugfixes: master...pps83:TurboPFor:master

[powturbo]

Thank you. I'll take a look at these changes.
To be honest, I don’t want to invest more time in TurboPFor right now — I’d just like it adapted to the newest compiler versions.
You’ve done a lot of great work on the TurboPFor repository. Are you using it in any real projects, or are you just interested in the topic?
By the way, is your integration of pcodec into icapp working?

[pps83]

I work on personal project and i use internally a few libs to avoid storing gigs of data. For PFor compression TurboPFor provides best result imo.
I don't recall details for pcodec, I recall some references to it were in the icapp, so I compiled its rust code. It was also ok, but imo TuroPFor is better.

To be honest, I don’t want to invest more time in TurboPFor right now — I’d just like it adapted to the newest compiler versions.

then ignore those comments above, these isn't trivial changes and there are 69 commits diff in my branch. When I have some time I'll try to make PRs for minor simple things that don't take u much time to review and merge

[powturbo]

I don't understand the stack-size issue. In the Makefile, a large stack is only required when the iccodecs module is included.
The only variables in the bitpack/bitunpack blocks are two vectors, ov and iv, of type __m128i or __m256i (16 or 32 bytes each), which are allocated on the stack.
We could move these vectors into the BITPACK128V32 / BITPACK256V32 macros to reduce stack usage. Where is the large stack size you're referring to actually coming from?

[pps83]

I didn't touch the code for a couple of years, very hard for me to remember.
In short, I had a few crashes in tests due to stackoverflow (I used regular stack). I disassembled all the code and there were all the stack allocations visible to see what functions required the most. Then, I run preprocess only to see these functions before they were compiled. There was nothing unusual about them, but it made compilers estimate huge amount of stack for some reason (I wanted to report a bug, but all compilers seem to have this bug :)
if you look into bitunpack256v32 function:

#if USEOLD
const unsigned char *bitunpack256v32( const unsigned char *__restrict in, unsigned n, unsigned *__restrict out, unsigned b) {
  const unsigned char *ip = in+PAD8(256*b);
  __m256i sv;
  BITUNPACK256V32(in, b, out, sv);
  return ip;
}
#else
BITUNPACK_DEFINE256(bitunpack256v32, (const unsigned char* __restrict in, unsigned n, unsigned* __restrict out),
  __m256i sv)
const unsigned char* bitunpack256v32(const unsigned char* __restrict in, unsigned n, unsigned* __restrict out, unsigned b) {
  const unsigned char* ip = in + PAD8(256 * b);
  BITUNPACK_CALL(bitunpack256v32, in, n, out);
  return ip;
}
#endif

in USEOLD branch this macro BITUNPACK256V32(in, b, out, sv); is the issue I think because it expands to huge switch, where every case [0..32] has some code, and that code block for each case is a macro that expands multiple times to big chunk of code. I think what my change does it creates 32 inline functions so that these blocks of code in switch aren't part of the same function. But these are forceinline functions, compiler still puts them into the bitunpack256v32 function, but stack estimation is already different when these blocks of code are outside of bitunpack256v32 function. This is my rough understanding of the change.

[powturbo]

When the bitpack/bitunpack files are compiled with the GCC option -fstack-usage, the compiler generates .su files containing the stack usage information for each function. These files show that the maximum stack usage for these functions is 488 bytes.
The only functions requiring significantly more stack space (up to 16 MB) belong to the context mixing functions in TurboRC.

[pps83]

pls, check same in debug builds, do they take huge amount of stack?

[powturbo]

Compiling with the -g (debug) option produces the same results.

[pps83]

I don't recall what I used for stack debugging. I think it's part of interla compiler fucntions. Something like ms_stack (or stack_ms) for msvc, and simply stack (or was it check_stach vs check_stam_ms) with gcc that shows up in asm output. I used compiler's asm output when building and extracted all functions with larges stack use. I wrote quick script to produce list of funcitons with stack sizes

[powturbo]

Ok, thank.
I've integrated your changes to include pcodec for testing.
Is it possible to get the cpcodec.so and cpcodec.dll files ?

[pps83]

Is it possible to get the cpcodec.so and cpcodec.dll files ?

i built it from https://github.com/mwlon/pcodec.git 0.3.1 (uploaded here)

[powturbo]

Thanks. pcodec has now been integrated and briefly tested on Windows.