P3: declarative ccbench_add_protocol() + move protocols under cc/ (closes #32) - #41
Merged
Conversation
) Each protocol's CMakeLists.txt collapses from ~70-130 lines of file(GLOB) + add_executable + add_definitions boilerplate into a single declarative call: ccbench_add_protocol(<name> SOURCES <shared .cc files> WORKLOADS ycsb tpcc bomb sbomb OPTIONS FOO=${CCBENCH_FOO} ...) - New cmake/ProtocolHelpers.cmake exposes ccbench_add_protocol() and a ccbench_write_protocol_matrix() that writes build/PROTOCOL_MATRIX.md from the WORKLOADS arg of every call — no more manual sync between CLAUDE.md / docs/protocols.md and the build system. - New cmake/Options.cmake centralises the universal -D flags as CACHE entries (CCBENCH_KEY_SIZE, CCBENCH_BACK_OFF, CCBENCH_VAL_SIZE, ...), replacing the ~180 directory-scoped add_definitions(...) calls duplicated across protocols. Override on the cmake command line: cmake -S . -B build -DCCBENCH_KEY_SIZE=16 -DCCBENCH_BACK_OFF=0 - target_compile_definitions(... PRIVATE ...) is used throughout, so flags no longer leak via directory scope. Protocol directories also move from <name>/ to cc/<name>/, grouping all CC implementations under one parent. Binaries now land in build/cc/<protocol>/<workload>_<protocol>.exe; docs and the runtime-args examples are updated to match. Source-file relative includes were shifted from ../include/... to ../../include/... to account for the extra depth. The helper deliberately does not call set_compile_options(): the old per-protocol CMakeLists never enabled -Wall -Wextra -Werror, the source tree has accumulated warnings under those flags, and turning -Werror on here would mix in a separate cleanup. Left as a follow-up.
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #32.
cmake/ProtocolHelpers.cmakeexposesccbench_add_protocol(<name> SOURCES ... WORKLOADS ... OPTIONS ...). Each protocol'sCMakeLists.txtcollapses from ~70-130 lines offile(GLOB)+add_executable+add_definitionsboilerplate into a single declarative call (6-19 lines).cmake/Options.cmakecentralises the universal-Dflags asCCBENCH_*cache entries (KEY_SIZE, BACK_OFF, VAL_SIZE, etc.), replacing the ~180 directory-scopedadd_definitions(...)calls duplicated across protocols. Override on the cmake command line:cmake -S . -B build -DCCBENCH_KEY_SIZE=16 -DCCBENCH_BACK_OFF=0.target_compile_definitions(... PRIVATE ...)— no more directory-scope leak.ccbench_write_protocol_matrix()dumpsbuild/PROTOCOL_MATRIX.mdfrom theWORKLOADSarg of every call, so the support table inCLAUDE.md/docs/protocols.mdcan be refreshed mechanically.<name>/tocc/<name>/. Binaries now land inbuild/cc/<protocol>/<workload>_<protocol>.exe; docs updated to match.Completion criteria (from #32)
CMakeLists.txtis ~10-20 linesadd_definitions(...)calls remainNotes / follow-ups
set_compile_options()(-Wall -Wextra -Werror). The old per-protocol CMakeLists never enabled-Werror, the source tree has accumulated warnings under those flags (ermia/scan_callback, masstree_wrapper, ...), and turning it on here would mix in a separate cleanup. Left as a follow-up.<protocol>.ccfiles (e.g.cc/cicada/cicada.cc) are legacy standalone YCSB mains that the old CMake silently excluded — kept in tree but not inSOURCES. Could be removed in a separate cleanup.occ/is still not wired into CMake; not moved undercc/this PR.Test plan
cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug && cmake --build build -jsucceeds locally (34 binaries)cmake -S . -B build-release -DCMAKE_BUILD_TYPE=Release -DENABLE_SANITIZER=OFF && cmake --build build-release -jsucceeds locally (34 binaries)./build/cc/silo/ycsb_silo.exe -thread_num=2 -extime=1 -clocks_per_us=2100shows both universal and silo-specific-Dflags applied