Numerous build and optimisation changes to speedup runtime performance#3
Numerous build and optimisation changes to speedup runtime performance#3doraemoncito merged 13 commits intomasterfrom
Conversation
3813634 to
55a69e8
Compare
…rous optimization changes to speedup runtime performance.
55a69e8 to
547a9f5
Compare
… keep header-only core, align tests and CMake, remove legacy src/z80.cpp
There was a problem hiding this comment.
Pull request overview
This pull request implements significant build system and performance improvements for the z80cpp Z80 emulator core. The primary goal is to optimize runtime performance while modernizing the build infrastructure.
Changes:
- Refactored from virtual dispatch to CRTP (Curiously Recurring Template Pattern) for zero-overhead polymorphism
- Introduced performance optimization macros (
Z80_FORCE_INLINE,Z80_LIKELY,Z80_RESTRICT) - Modernized CMake build system with LTO, ccache support, and parallel builds
- Added code quality tooling (clang-format, clang-tidy, GitHub Actions CI)
- Renamed header files from
.hppto.hfor C++ consistency - Updated benchmark tests with higher performance expectations
Reviewed changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/z80_sim_test.h | New header with updated class interface using CRTP pattern |
| tests/z80_sim_test.cpp | Refactored to use CRTP bus interface, fixed timing issue, removed Unicode symbols |
| tests/z80_game_test.cpp | Simplified using C++17 filesystem, updated performance thresholds |
| tests/z80_benchmark_test.cpp | Updated performance expectations to match optimized build |
| tests/benchmark_shared.hpp | Converted to shim forwarding to .h file |
| tests/benchmark_shared.h | New implementation using CRTP pattern |
| include/z80_types.h | New header defining performance macros and RegisterPair type |
| include/z80_bus_interface.h | New CRTP-based bus interface replacing virtual dispatch |
| include/z80operations.h | Removed (replaced by z80_bus_interface.h) |
| CMakeLists.txt | Complete modernization with optimizations and tooling support |
| .clang-format | Added code formatting configuration |
| .clang-tidy | Added static analysis configuration |
| .github/workflows/code-quality.yml | Added CI workflow for code quality checks |
| README.md | Extensive documentation updates with build instructions and performance data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… keep header-only core, align tests and CMake, remove legacy src/z80.cpp
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
tests/z80_benchmark_test.cpp:1
- Performance thresholds increased dramatically from 0.1 MIPS to 80-100 MIPS without verification that the optimizations actually achieve this. These high thresholds may cause tests to fail if the claimed 80% speedup isn't realized in all environments. Consider using more conservative thresholds initially or making them configurable.
// Z80 Performance Benchmark Test Suite
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… keep header-only core, align tests and CMake, remove legacy src/z80.cpp
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
tests/z80_benchmark_test.cpp:1
- The performance thresholds have been dramatically increased from 0.1 MIPS to 80-100 MIPS. While these reflect optimized build expectations, they may cause test failures on slower systems or debug builds. Consider adding conditional thresholds based on CMAKE_BUILD_TYPE or documenting that these tests are only meaningful for Release builds.
// Z80 Performance Benchmark Test Suite
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This change set implements significant build system and performance improvements for the z80cpp Z80 emulator core. The primary goal is to optimise runtime performance while modernising the build infrastructure.
Changes:
Z80_FORCE_INLINE,Z80_LIKELY,Z80_RESTRICT).hppto.hfor C++ consistency