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

Implement CallProfiler #258

Closed
wants to merge 3 commits into from
Closed

Conversation

tigercosmos
Copy link
Collaborator

@tigercosmos tigercosmos commented Dec 2, 2023

This work is based on the previous #245

This work introduces a new profiler, called CallProfiler, which can print the profiling result of a hierarchical caller stack.

The example output looks like this (please refer to test_nopython_callprofiler.cpp):

constexpr int uniqueTime1 = 19;
constexpr int uniqueTime2 = 35;
constexpr int uniqueTime3 = 7;

void foo3()
{
    modmesh::USE_CALLPROFILER_PROFILE_THIS_FUNCTION();
    std::this_thread::sleep_for(std::chrono::milliseconds(uniqueTime1));
}

void foo2()
{
    modmesh::USE_CALLPROFILER_PROFILE_THIS_FUNCTION();
    std::this_thread::sleep_for(std::chrono::milliseconds(uniqueTime2));
    foo3();
}

void foo1()
{
    modmesh::USE_CALLPROFILER_PROFILE_THIS_FUNCTION();
    foo2();
    std::this_thread::sleep_for(std::chrono::milliseconds(uniqueTime3));
}


int main()
{
    auto & profiler = modmesh::Profiler::instance();

    foo1();
    foo2();
    foo3();
    foo3();
  
    profiler.print_profiling_result();
}
Profiling Result
  foo1 - Total Time: 61 ms, Call Count: 1
    foo2 - Total Time: 54 ms, Call Count: 1
      foo3 - Total Time: 19 ms, Call Count: 1
  foo2 - Total Time: 54 ms, Call Count: 1
    foo3 - Total Time: 19 ms, Call Count: 1
  foo3 - Total Time: 38 ms, Call Count: 2

cc @q40603


void foo3()
{
modmesh::USE_CALLPROFILER_PROFILE_THIS_FUNCTION();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we use the macro here to insert the profiler.

@tigercosmos
Copy link
Collaborator Author

The implementation is almost there. I will fix the CI once the proposal is accepted.

foo3 - Total Time: 19 ms, Call Count: 1
)";

EXPECT_EQ(ss.str(), answer);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just for demonstration here, I think it's better to compare each node one by one.

@yungyuc
Copy link
Member

yungyuc commented Dec 2, 2023

The implementation is almost there. I will fix the CI once the proposal is accepted.

Let me add comments after the feedback from @q40603 .

But I would like the code review to happen after CI is clean. Fixing CI needs to change code, which requires additional review. It does not make sense to ask for review before making CI clean.

modmesh CI includes linter. It wastes precious reviewing resources when the linter automatically tells you there are things needing to be fixed.

@yungyuc yungyuc added the performance Profiling, runtime, and memory consumption label Dec 2, 2023
@tigercosmos
Copy link
Collaborator Author

I would like to have more discussion on this PR, let me close this one can create a new draft PR.

@tigercosmos tigercosmos closed this Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Profiling, runtime, and memory consumption
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants