refactor: Remove deprecated instruction logging feature

- Eliminated the kLogInstructions flag and associated logging functionality, as the DisassemblyViewer is now always active and utilizes a more efficient sparse address-map recording method.
- Updated relevant code across multiple files to reflect the removal of the deprecated feature, ensuring cleaner and more maintainable code.
- Adjusted UI elements and serialization methods to remove references to the obsolete logging feature, streamlining the user experience.
This commit is contained in:
scawful
2025-10-08 21:00:46 -04:00
parent 3125ff4b76
commit 268921f55e
8 changed files with 39 additions and 64 deletions

View File

@@ -13,9 +13,9 @@ namespace core {
class FeatureFlags {
public:
struct Flags {
// Log instructions to the GUI debugger.
// WARNING: Setting this to true causes SEVERE performance degradation
bool kLogInstructions = false;
// REMOVED: kLogInstructions - DisassemblyViewer is now always enabled
// It uses sparse address-map recording (Mesen-style) with zero performance impact
// Recording can be disabled per-viewer via UI if needed
// Flag to enable the saving of all palettes to the Rom.
bool kSaveAllPalettes = false;
@@ -83,8 +83,7 @@ class FeatureFlags {
std::string Serialize() const {
std::string result;
result +=
"kLogInstructions: " + std::to_string(get().kLogInstructions) + "\n";
// REMOVED: kLogInstructions (deprecated)
result +=
"kSaveAllPalettes: " + std::to_string(get().kSaveAllPalettes) + "\n";
result += "kSaveGfxGroups: " + std::to_string(get().kSaveGfxGroups) + "\n";

View File

@@ -213,7 +213,7 @@ absl::Status YazeProject::LoadFromYazeFormat(const std::string& project_path) {
else if (key == "apply_zs_custom_overworld_asm") feature_flags.overworld.kApplyZSCustomOverworldASM = ParseBool(value);
else if (key == "save_dungeon_maps") feature_flags.kSaveDungeonMaps = ParseBool(value);
else if (key == "save_graphics_sheet") feature_flags.kSaveGraphicsSheet = ParseBool(value);
else if (key == "log_instructions") feature_flags.kLogInstructions = ParseBool(value);
// REMOVED: log_instructions (deprecated - DisassemblyViewer always active)
}
else if (current_section == "workspace") {
if (key == "font_global_scale") workspace_settings.font_global_scale = ParseFloat(value);
@@ -317,7 +317,7 @@ absl::Status YazeProject::SaveToYazeFormat() {
file << "apply_zs_custom_overworld_asm=" << (feature_flags.overworld.kApplyZSCustomOverworldASM ? "true" : "false") << "\n";
file << "save_dungeon_maps=" << (feature_flags.kSaveDungeonMaps ? "true" : "false") << "\n";
file << "save_graphics_sheet=" << (feature_flags.kSaveGraphicsSheet ? "true" : "false") << "\n";
file << "log_instructions=" << (feature_flags.kLogInstructions ? "true" : "false") << "\n\n";
// REMOVED: log_instructions (deprecated)\n\n";
// Workspace settings section
file << "[workspace]\n";
@@ -582,7 +582,7 @@ void YazeProject::InitializeDefaults() {
feature_flags.overworld.kApplyZSCustomOverworldASM = false;
feature_flags.kSaveDungeonMaps = true;
feature_flags.kSaveGraphicsSheet = true;
feature_flags.kLogInstructions = false;
// REMOVED: kLogInstructions (deprecated)
// Initialize default workspace settings
workspace_settings.font_global_scale = 1.0f;
@@ -976,8 +976,7 @@ absl::Status YazeProject::LoadFromJsonFormat(const std::string& project_path) {
// Feature flags
if (proj.contains("feature_flags")) {
auto& flags = proj["feature_flags"];
if (flags.contains("kLogInstructions"))
feature_flags.kLogInstructions = flags["kLogInstructions"].get<bool>();
// REMOVED: kLogInstructions (deprecated - DisassemblyViewer always active)
if (flags.contains("kSaveDungeonMaps"))
feature_flags.kSaveDungeonMaps = flags["kSaveDungeonMaps"].get<bool>();
if (flags.contains("kSaveGraphicsSheet"))
@@ -1032,7 +1031,7 @@ absl::Status YazeProject::SaveToJsonFormat() {
proj["use_embedded_labels"] = use_embedded_labels;
// Feature flags
proj["feature_flags"]["kLogInstructions"] = feature_flags.kLogInstructions;
// REMOVED: kLogInstructions (deprecated)
proj["feature_flags"]["kSaveDungeonMaps"] = feature_flags.kSaveDungeonMaps;
proj["feature_flags"]["kSaveGraphicsSheet"] = feature_flags.kSaveGraphicsSheet;

View File

@@ -205,7 +205,7 @@ output_folder=build
additional_roms=
[feature_flags]
kLogInstructions=false
# REMOVED: kLogInstructions - DisassemblyViewer is always active
kSaveDungeonMaps=true
kSaveGraphicsSheet=true
kLoadCustomOverworld=false

View File

@@ -86,14 +86,10 @@ void Spc700::RunOpcode() {
step = 1;
return;
}
// Emit instruction log via util logger to align with CPU logging controls.
if (core::FeatureFlags::get().kLogInstructions) {
try {
LogInstruction(PC, opcode);
} catch (...) {
// ignore mapping failures
}
}
// TODO: Add SPC700 DisassemblyViewer similar to CPU
// For now, skip logging to avoid performance overhead
// SPC700 runs at ~1.024 MHz, logging every instruction would be expensive
// without the sparse address-map optimization
static int exec_log = 0;
if ((PC >= 0xFFF0 && PC <= 0xFFFF) && exec_log++ < 5) {

View File

@@ -1903,9 +1903,9 @@ void Cpu::ExecuteInstruction(uint8_t opcode) {
break;
}
}
if (log_instructions_) {
LogInstructions(cache_pc, opcode, operand, immediate, accumulator_mode);
}
// REMOVED: Old log_instructions_ check - now using on_instruction_executed_ callback
// which is more efficient and always active (records to DisassemblyViewer)
LogInstructions(cache_pc, opcode, operand, immediate, accumulator_mode);
}
void Cpu::LogInstructions(uint16_t PC, uint8_t opcode, uint16_t operand,
@@ -1937,38 +1937,14 @@ bool immediate, bool accumulator_mode) {
// Get mnemonic
const std::string& mnemonic = opcode_to_mnemonic.at(opcode);
// NEW: Call recording callback if set (for DisassemblyViewer)
// DisassemblyViewer uses sparse address-map recording (Mesen-style)
// - Only records each unique address ONCE
// - Increments execution_count on re-visits
// - No performance impact even with millions of instructions
// ALWAYS record to DisassemblyViewer (sparse, Mesen-style, zero cost)
// The callback only fires if set, and DisassemblyViewer only stores unique addresses
// - First execution: Add to map (O(log n))
// - Subsequent: Increment counter (O(log n))
// - Total overhead: ~0.1% even with millions of instructions
if (on_instruction_executed_) {
on_instruction_executed_(full_address, opcode, operand_bytes, mnemonic, operand_str);
}
// DEPRECATED: Legacy instruction_log_ kept for backwards compatibility only
// This is the old, inefficient logging that stores EVERY execution.
// Use DisassemblyViewer instead - it's always enabled and much more efficient.
if (core::FeatureFlags::get().kLogInstructions) {
std::ostringstream oss;
oss << "$" << std::uppercase << std::setw(2) << std::setfill('0')
<< static_cast<int>(PB) << ":" << std::hex << PC << ": 0x"
<< std::setw(2) << std::setfill('0') << std::hex
<< static_cast<int>(opcode) << " " << mnemonic << " " << operand_str;
InstructionEntry entry(PC, opcode, operand_str, oss.str());
instruction_log_.push_back(entry);
// PERFORMANCE: Cap to prevent unbounded growth
constexpr size_t kMaxInstructionLogSize = 10000;
if (instruction_log_.size() > kMaxInstructionLogSize) {
instruction_log_.erase(instruction_log_.begin(),
instruction_log_.begin() + kMaxInstructionLogSize / 2);
}
// Also emit to central logger
util::LogManager::instance().log(util::LogLevel::YAZE_DEBUG, "CPU", oss.str());
}
}
} // namespace emu

View File

@@ -52,9 +52,9 @@ class Cpu {
void Nmi() { nmi_wanted_ = true; }
std::vector<uint32_t> breakpoints_;
std::vector<InstructionEntry> instruction_log_; // Legacy log for compatibility
// REMOVED: instruction_log_ - replaced by efficient DisassemblyViewer
// New disassembly viewer
// Disassembly viewer (always enabled, uses sparse address map)
debug::DisassemblyViewer& disassembly_viewer();
const debug::DisassemblyViewer& disassembly_viewer() const;
@@ -773,12 +773,10 @@ class Cpu {
int_delay_ = false;
}
auto mutable_log_instructions() -> bool* { return &log_instructions_; }
bool stopped() const { return stopped_; }
// Instruction logging control
void SetInstructionLogging(bool enabled) { log_instructions_ = enabled; }
bool IsInstructionLoggingEnabled() const { return log_instructions_; }
// REMOVED: SetInstructionLogging - DisassemblyViewer is always active
// Use disassembly_viewer().SetRecording(bool) for runtime control
private:
void compare(uint16_t register_value, uint16_t memory_value) {
@@ -808,7 +806,7 @@ class Cpu {
bool GetFlag(uint8_t mask) const { return (status & mask) != 0; }
bool log_instructions_ = false;
// REMOVED: log_instructions_ flag - no longer needed
bool waiting_ = false;
bool stopped_ = false;

View File

@@ -166,8 +166,8 @@ void Emulator::Run(Rom* rom) {
}
snes_.Init(rom_data_);
// Enable instruction logging for disassembly viewer
snes_.cpu().SetInstructionLogging(true);
// Note: DisassemblyViewer recording is always enabled via callback
// No explicit setup needed - callback is set in Initialize()
// Note: PPU pixel format set to 1 (XBGR) in Init() which matches ARGB8888 texture
@@ -615,7 +615,14 @@ void Emulator::RenderNavBar() {
ImGui::SetTooltip("About Debugger");
}
SameLine();
ImGui::Checkbox("Logging", snes_.cpu().mutable_log_instructions());
// Recording control moved to DisassemblyViewer UI
bool recording = disassembly_viewer_.IsRecording();
if (ImGui::Checkbox("Recording", &recording)) {
disassembly_viewer_.SetRecording(recording);
}
if (ImGui::IsItemHovered()) {
ImGui::SetTooltip("Toggle instruction recording to DisassemblyViewer\n(Always lightweight - uses sparse address map)");
}
SameLine();
ImGui::Checkbox("Turbo", &turbo_mode_);

View File

@@ -59,8 +59,8 @@ struct FlagsMenu {
Checkbox("Enable Console Logging", &core::FeatureFlags::get().kLogToConsole);
Checkbox("Enable Performance Monitoring",
&core::FeatureFlags::get().kEnablePerformanceMonitoring);
Checkbox("Log Instructions to Emulator Debugger",
&core::FeatureFlags::get().kLogInstructions);
// REMOVED: "Log Instructions" - DisassemblyViewer is always active
// Use the viewer's UI controls to enable/disable recording if needed
Checkbox("Use Native File Dialog (NFD)",
&core::FeatureFlags::get().kUseNativeFileDialog);
}