Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add profiling support #1583

Open
wants to merge 1 commit into
base: extui-align
Choose a base branch
from
Open

Conversation

cgromulo
Copy link

@cgromulo cgromulo commented Jan 9, 2025

Add support for profiling SH2 code. The profiler works by keeping track of sub-routine jumps with PC addresses inside a certain range (by default between 0x06002000 and 0x06104000). It allows uses to activate/deactivate the profiler by writing commands to the dev cart (range can be selected if users push the start and end addresses first to the newly created command stack). Emulator users can then show, export and clear the results (as well as toggle profiling) from a new sub-menu on 'Tools'.

PS: I tried my best to adhere to yabause/kronos formatting but it is very chaotic in some places. Hopefully we can agree to use something like clang-format in the future to make this kind of contribution even easier.

Critics and comments are welcome anyway :)


//////////////////////////////////////////////////////////////////////////////

static u8 FASTCALL SH2ProfilerTrackAddr(u32 addr, SH2_struct* sh)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the code here and executing it in the interpreter itself has a real performance impact on user lambda which never use the feature. Every simple function call presents an overhead. It should be better to handle it at a upper level, like breakpoints are done and only allow (and show) the option when a compatible cart is plugged. On upper level their might be a debug interpreter with its own set of function and norlmally, it already handle the subbroutine jump and return

@@ -1,21 +1,21 @@
/* Copyright 2005 Guillaume Duhamel
Copyright 2005-2006, 2013 Theo Berkau
Copyright 2008 Filipe Azevedo <[email protected]>
Copyright 2005-2006, 2013 Theo Berkau
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting is a mess in kronos, it is lmainly due to the fact I am working in linux, with default formatting rules.
So unless there is a huge rework of the full formatting, if I accept this reformatting in the PR, it will always be a ping pong in future PR.
As this formatting stuff is reducing visibility on what has to change really, is that possible to split the patches in two patches. First one: open Kronos source and save them with your IDE, it will generate formatting difference that you can add as a separate patch, then goes to your effective changes in a second patch so that review will be easier?

@@ -1169,6 +1170,25 @@ void UIYabause::on_aViewDebugMemoryEditor_triggered()
UIMemoryEditor( UIDebugCPU::PROC_MSH2, mYabauseThread, this ).exec();
}

void UIYabause::on_aProfilerToggle_triggered() {
YabauseLocker locker( mYabauseThread );
if (mYabauseThread->init() == 0 && MSH2 && SSH2) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might handle caartdev detection here or only enable the option widget when dev cart has been selected.

@@ -238,6 +241,9 @@ int SH2Init(int coreid)
if (SH2TrackInfLoopInit(MSH2) != 0)
return -1;

if (SH2ProfilerInit(MSH2) != 0)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only enable this on DEBUG Core

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.

3 participants