Skip to content

Guarantee compile-time initialization of scan arena tables - #311

Merged
thweetkomputer merged 1 commit into
eloqdata:mainfrom
thweetkomputer:pr/scan-arena-constexpr-530faea2
Oct 11, 2026
Merged

thweetkomputer merged 1 commit into
eloqdata:mainfrom
thweetkomputer:pr/scan-arena-constexpr-530faea2

Conversation

@thweetkomputer

Copy link
Copy Markdown
Collaborator

Context

The scan arena size-class tables should be guaranteed to initialize at compile time. This PR contains only commit 530faea27c738b89a4a10e303c41e456118c7001.

Behavior before and after

Previously, the tables were defined as inline const, and the lookup builder read kClassSizes, leaving constant initialization without an explicit language-level guarantee. Both definitions are now inline constexpr, so the compiler enforces constant initialization. Size-class values and allocation behavior remain unchanged.

Implementation

BuildClassLookup() builds a local constexpr size array using BuildClassSizes(). Both table definitions use inline constexpr.

Design decisions and alternatives

Use compile-time evaluation of the existing builders, preserving their shared size-class calculation and avoiding runtime initialization dependencies.

Documentation and comments

No architectural claim, public API behavior, ownership model, or durable format changes; no architecture update is required. The change is confined to the table builder and definitions.

Test plan

Passed in an isolated worktree on ARM64 with GCC 13.3 and Clang 18.1:

git diff 633307fead5afae81a3498cc2f15c079c1095910..HEAD --check
printf '#include "lavik/storage/scan_hash_map.h"\n' | g++ -std=c++23 -Iinclude -I/home/ubuntu/lavik/third_party/mimalloc/include -I/home/ubuntu/lavik/bycorf/third_party/abseil -x c++ -fsyntax-only -
printf '#include "lavik/storage/scan_hash_map.h"\n' | clang++ -std=c++23 -Iinclude -I/home/ubuntu/lavik/third_party/mimalloc/include -I/home/ubuntu/lavik/bycorf/third_party/abseil -x c++ -fsyntax-only -

The compiler checks use the existing dependency headers read-only. Full build and runtime tests were not run locally.

Risk assessment

Low: the size-class calculation is unchanged. Both tested compilers accept the constexpr definitions and evaluate their initializers successfully.

Rollback plan

Revert this commit.

Reviewer guide

Review BuildClassLookup() and the two table definitions in include/lavik/storage/scan_hash_map.h.

Follow-up work

None planned.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f24c13e9-3491-4515-b211-976245946b86

📥 Commits

Reviewing files that changed from the base of the PR and between 633307f and 530faea.


📒 Files selected for processing (1)
  • include/lavik/storage/scan_hash_map.h

 ___________________________________________________________________________
< I love a good shortcut. This is a great shortcut to production incidents. >
 ---------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thweetkomputer
thweetkomputer merged commit 1eacae5 into eloqdata:main Oct 11, 2026
5 of 6 checks passed
@thweetkomputer
thweetkomputer deleted the pr/scan-arena-constexpr-530faea2 branch October 11, 2026 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant