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 continuous collection of profiles in java profiler #730

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

marcin-ol
Copy link
Collaborator

Add more accurate collection of java application profiles using async-profiler's dump command.

Description

This PR changes the way that async-profiler is invoked.
From now on gprofiler starts ap once and periodically issues dump command to retrieve the collapsed profiles.

Related Issue

#569

Motivation and Context

More accurate profiling and better performance due to having async-profiler loaded in process.

How Has This Been Tested?

Whole collection of java tests in repository was executed against this change.

Checklist:

  • I have read the CONTRIBUTING document.
  • I have updated the relevant documentation.
  • I have added tests for new logic.

@marcin-ol
Copy link
Collaborator Author

marcin-ol commented Mar 20, 2023

Several things need addressing (and the solution is underway):

  • async-profiler timeout needs to be reapplied once async-profiler gets ability to reset the timeout timer,
  • adjust output for cumulative frame count - async-profiler doesn't reset it, frame counts are increasing,
  • account for dump skew - time between last dump output and the next call to snapshot().

(these are no longer needed, handled in async-profiler and in gProfiler, respectively).

@marcin-ol
Copy link
Collaborator Author

Java profiling is now truly continuous thanks to async-profiler timeout recycle functionality:
Granulate/async-profiler#6 .

@marcin-ol
Copy link
Collaborator Author

Last commit got pushed too early. Adjustment is correct in principle, but needs to take multiple profilers into account.

@marcin-ol
Copy link
Collaborator Author

Output from profiler in continuous mode is now equivalent to output produced by profiler on master. Frame counts now cover only the snapshot window.

@marcin-ol marcin-ol marked this pull request as ready for review March 27, 2023 07:54
@Jongy Jongy added enhancement New feature or request runtime/java labels Jul 8, 2023
Copy link
Contributor

@Jongy Jongy left a comment

Choose a reason for hiding this comment

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

Please merge master & update async-profiler to the latest revision used in master, I'll get to reviewing this soon.

@marcin-ol
Copy link
Collaborator Author

Please merge master & update async-profiler to the latest revision used in master, I'll get to reviewing this soon.

Done.

@marcin-ol marcin-ol requested a review from Jongy July 14, 2023 12:54
@marcin-ol
Copy link
Collaborator Author

Added resettrace flag to async-profiler, which resets frame counts after dump. This removes need for adjusting collected stacks, mentioned in #730 (comment).

@marcin-ol
Copy link
Collaborator Author

Also, adjusting for time skew is not needed. gProfiler does it automatically, when merging collected stacks from runtime profilers with system profiler.

@marcin-ol
Copy link
Collaborator Author

marcin-ol commented Jul 20, 2023

One remaining task:

  • make async-profiler even more continuous, removing need to call status between dump calls.

@marcin-ol
Copy link
Collaborator Author

Implemented smarter error handling in async-profiler with dumpactive flag. Command dump will now report error if profiler is stopped, letting java profiler know that it needs to restart profiling.
This removes need for status calls, making java profiling more fluent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request runtime/java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants