refactor(chain): extract hardfork application into database_hardfork.cpp - #149
refactor(chain): extract hardfork application into database_hardfork.cpp#149chiliec wants to merge 2 commits into
Conversation
database.cpp is the single longest-to-compile translation unit in the project (~8,000 lines, one object file, cannot be parallelized). This moves the self-contained hardfork cluster into its own TU so it compiles in parallel with the rest of database.cpp and edits to hardfork logic no longer force a recompile of the entire file. Moved verbatim (byte-for-byte) from database.cpp into database_hardfork.cpp: init_hardforks, reset_virtual_schedule_time, process_hardforks, has_hardfork, set_hardfork, apply_hardfork (the contiguous block that was lines 7012-7989). database.cpp shrinks by ~975 lines (7991 -> 7016); the new file is ~1,020 lines. No behavior change: - Method bodies are identical; only their file location changed. - The cluster touches only database members declared in database.hpp (_hardfork_times, _hardfork_versions, _log_hardforks), so a separate TU can define them. CHAIN_HARDFORK_*/HARDFORK_* constants come from the generated hardfork.hpp, which database.hpp includes, so the new file (which includes database.hpp first) sees them. - The two VIRTUAL_SCHEDULE_LAP_LENGTH* macros are still used elsewhere in database.cpp (lines ~3348, ~3817), so they remain defined there; the new file gets its own copy because reset_virtual_schedule_time needs them. Kept in sync by a comment in both places. - to256() is a small inline helper already duplicated per-TU (database.cpp and chain_evaluator.cpp both define it); the new TU gets the same local copy. - Added database_hardfork.cpp to both the SHARED and STATIC source lists in libraries/chain/CMakeLists.txt. The existing add_dependencies(graphene_chain ... build_hardfork_hpp) already covers the new TU's need for the generated header. Verification (full tree build not possible in my env - no Boost/submodules): - Symbol accounting: 163 database:: definitions/references before == 163 after (db + hardfork), none lost, none added. - The original 7012-7989 block appears verbatim in the new file. - Both files are brace-balanced; namespace nesting is closed once. - The added scaffolding (the two macros, the to256 helper, u256 usage, namespace) passes g++ -std=c++14 -fsyntax-only against minimal stubs.
CI caught a real compile error I could not reproduce locally (no Boost/ submodules in my env): apply_hardfork references proposal_index and required_approval_index (from proposal_object.hpp), which my hand-picked include list omitted -> 'proposal_index was not declared in this scope'. Replace the partial include list with the full graphene/chain/* set that database.cpp itself uses for the hardfork path (proposal_object.hpp, block_summary_object.hpp, transaction_object.hpp, index.hpp, db_with.hpp, evaluator_registry.hpp, operation_notification.hpp, chain_evaluator.hpp, compound.hpp, shared_db_merkle.hpp). Since these methods previously compiled in database.cpp with exactly this include surface, matching it guarantees every object-index type they touch is visible. Verified the new file's graphene/chain include set is now a superset-equal of database.cpp's; braces balanced; namespace closed once.
…cpp (pm, HF14-preserving) Native re-implementation of #149 on the pm branch. #149 as authored is based on master and its extracted database_hardfork.cpp does NOT contain pm's HF14 wiring (init_hardforks registers CHAIN_HARDFORK_14 time/version; apply_hardfork's CHAIN_HARDFORK_14 case instantiates the pm_lazy_pool_object singleton). Merging #149 verbatim would have silently dropped that consensus code. Instead this moves pm's OWN hardfork functions (init_hardforks, reset_virtual_schedule_time, process_hardforks, has_hardfork, set_hardfork, apply_hardfork) verbatim from database.cpp into database_hardfork.cpp. Verified pure move: the 987 moved lines are byte-identical (diff clean). Added pm_objects.hpp to the mirrored include set (HF14 apply case needs pm_lazy_pool_object). Both TUs pass single-TU -fsyntax-only.
|
Folded into the
Merging this PR as-is into So on Same idea as your PR, HF14 preserved — thanks for it! When |
First step of splitting the
database.cppgod file (F001 in the tech-debt audit). This is a pure file-level extraction — no behavior change.Why
libraries/chain/database.cppis ~8,000 lines compiled into a single object file — the longest-to-compile TU in the project, and it can't be parallelized (the chain CMake even comments "database takes the longest to compile, start it first"; MSVC needs/bigobjfor it). Moving a self-contained cluster into its own TU lets it compile in parallel and stops hardfork edits from forcing a recompile of the whole file.I deliberately started with the smallest, most self-contained, provably-bounded cluster rather than attempting the whole split at once.
What moved
The contiguous hardfork block (originally lines 7012–7989) moved verbatim into
libraries/chain/database_hardfork.cpp:init_hardforksreset_virtual_schedule_timeprocess_hardforkshas_hardforkset_hardforkapply_hardforkdatabase.cppshrinks ~975 lines (7991 → 7016); the new file is ~1,020 lines.Why it's safe
database.hpp(_hardfork_times,_hardfork_versions,_log_hardforks), so a separate TU can define these methods.CHAIN_HARDFORK_*/HARDFORK_*constants come from the generatedhardfork.hpp, whichdatabase.hppincludes; the new file includesdatabase.hppfirst, so it sees them.VIRTUAL_SCHEDULE_LAP_LENGTH*macros are still used elsewhere indatabase.cpp(≈ lines 3348, 3817), so they stay defined there. The new file gets its own copy (needed byreset_virtual_schedule_time), with a sync note in both places.to256()is a tiny inline helper already duplicated per-TU (database.cppandchain_evaluator.cppeach define it); the new TU gets the same local copy.database_hardfork.cppto both the SHARED and STATIC source lists. Existingadd_dependencies(graphene_chain ... build_hardfork_hpp)already covers the new TU's need for the generated header.Verification
I don't have Boost dev libs or the submodules in my environment, so a full tree build wasn't possible. What I did verify:
database::definitions/references before == 163 after (database.cpp + database_hardfork.cpp) — none lost, none added.to256helper,u256usage, namespace) passesg++ -std=c++14 -fsyntax-onlyagainst minimal stubs.Ask for a maintainer: please confirm a full
-DBUILD_TESTNET=OFFbuild links clean on a box with the real deps before merge. The extraction is mechanical and symbol-accounted, but I couldn't run the final compile+link here.If this lands cleanly, the same pattern extends to the other natural seams the audit names (apply-block path, snapshot integration, undo-session lifecycle).