serge-sans-paille / frozen

a header-only, constexpr alternative to gperf for C++14 users
Apache License 2.0
1.31k stars 104 forks source link

feat: zero relocation string containers #159

Open muggenhor opened 1 year ago

muggenhor commented 1 year ago

This makes the underlying storage type, used by the containers and previously hardcoded to bits::carray, configurable.

It then adds a specialized container (pic_array) for storing variable-length arrays of Ts. The most prominent example of such arrays being strings (arrays of char). This container stores these arrays as sub-arrays inside one large storage array.

This simultaneously achieves:

  1. higher density than over-allocating to fit the largest array (string)
  2. better memory locality than storing (pointer,size) tuples (aka span/string_view) or over-allocating
  3. unlike storing pointers (span/stringview) this avoids `R_RELATIVE-style *load-time* relocations (for-fPIC`) because it stores relative addresses to data (indices into the storage array) instead of absolute addresses
  4. consumes less (rodata) memory than storing pointers by selecting the smallest possible integer capable of addressing the entire storage array for storing relative addresses

I have two different use cases where this benefits me significantly:

  1. huge lookup tables in shared libraries that cause noticeable delays at process startup (20ms average for the largest) when the dynamic linker (ld.so) is relocating large amounts of string pointers in .data.rel.ro: these get moved to .rodata which doesn't need to be relocated at all.
  2. different project on an ESP8266 where the increased memory density allows me to shave off ~15 kB of precious flash storage.

Depends-on: #158

muggenhor commented 1 year ago

Well, I was not expecting those build failures on MSVC. I'll try to resolve those tomorrow.

muggenhor commented 1 year ago

It appears I've managed to discover some bugs in MSVC. Hopefully my last commit successfully works around them (I may still have to add or subtract one level of std::enable_if SFINAE usage). I need to wait for a subsequent AppVeyor run to be able to confirm as I don't have MSVC myself. (And Compiler Explorer only goes so far).

I've had some discussions in the CppLang slack about this https://cpplang.slack.com/archives/C21PKDHSL/p1692113981628139 (invite, if required, here: https://cppalliance.org/slack/). It appears that MSVC is definitely buggy with some combination of parameter packs, NTTPs (non-type template parameters) and reference-to-bounded arrays. There appear to be multiple ways to work around this. I've gone for the one that seemed most palatable to me (adding and using wrapper type array_ref<T, N> as replacement for const T(&)[N]).

muggenhor commented 1 year ago

I finally managed to get a Windows VM with VS 2017 (because AppVeyor runs with 2017). After 5352523e1c23e5b6d9511c6e391a7961750f3fcd I don't have any build errors locally anymore. So hopefully AppVeyor concurs...

muggenhor commented 1 year ago

@serge-sans-paille I think you need to trigger the build on AppVeyor? Could you do that (or, if I'm wrong, tell my how to do so myself).

serge-sans-paille commented 1 year ago

@muggenhor I've removed appveyor and travis CI in favor of github actions, could you rebase on master branch first?

muggenhor commented 1 year ago

I had to do some changes to get CI to work:

  1. 9c3d62a61e6f40d31df87ddebd9687aea774386f: necessary to trigger at all
  2. a9576b30a11b4821645886e6384aa4fc6d395737: to get usable workflow names in the GitHub Actions tab
  3. 145cb94f71d2b5c43be5dad47633532b229d46a8: get colored error messages

(2) and (3) are not strictly necessary. So I can remove them easily enough if you dislike those changes.

Also I used interactive rebase to make them the commits closest to master. This allows you to merge them straight to master right now if you would want to (git merge --ff-only --ff 145cb94f71d2b5c43be5dad47633532b229d46a8).

serge-sans-paille commented 1 year ago

I started reviewing the changes... but there are too many of them. Could you split this in individual PRs I can review independently? The time I have to spend on frozen is unfortunately limited :'(

muggenhor commented 1 year ago

I can split it, but there's dependencies between the changes, so there are some limits to how far I can split it.

Either way, I'm also somewhat restricted in time. I was hoping to do some splitting this week but didn't end up having any time. Hopefully somewhere in the next two weeks.

agraham-roku commented 8 months ago

I came here looking for exactly this, thank you.

Another good thing to have for unordered_map/unordered_set would be the option to simply not store the key values in addition to the tables. This means it won't be able to detect a collision and a failed lookup will return an arbitrary result, but in practice, sometimes that's ok.