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

Backend self sampling #8309

Merged
merged 13 commits into from
Nov 17, 2023
Merged

Backend self sampling #8309

merged 13 commits into from
Nov 17, 2023

Conversation

4e6
Copy link
Contributor

@4e6 4e6 commented Nov 15, 2023

Pull Request Description

close #8248

Changelog:

  • add: profiling/start request starts the sampler and starts collecting runtime events to the log file
  • add: profiling/stop request stop the sampler and write the profiling data to the $ENSO_DATA_DIR/profiling directory
  • refactor: rewrite the profiling logic into Java

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@4e6 4e6 added the CI: No changelog needed Do not require a changelog entry for this PR. label Nov 15, 2023
@4e6 4e6 self-assigned this Nov 15, 2023
@4e6 4e6 added the CI: Ready to merge This PR is eligible for automatic merge label Nov 17, 2023
@mergify mergify bot merged commit a286ab7 into develop Nov 17, 2023
32 of 33 checks passed
@mergify mergify bot deleted the wip/db/8248-backend-self-sampling branch November 17, 2023 15:04
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;

public interface MethodsSampler {
Copy link
Member

@JaroslavTulach JaroslavTulach Nov 18, 2023

Choose a reason for hiding this comment

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

sealed interface is probably what you want with the two implementations being permits listed.

public void start() {}

@Override
public void stop() {}
Copy link
Member

Choose a reason for hiding this comment

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

scheduleStop could be no-op either.

* Gathers application performance statistics that can be visualised in Java VisualVM, and writes it
* to the provided output.
*/
public final class OutputStreamSampler implements MethodsSampler {
Copy link
Member

Choose a reason for hiding this comment

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

To keep the API's conceptual surface smaller, I'd suggest to make these implementations package private and only expose MethodsSampler.

}

public static OutputStreamSampler ofFile(File file) throws FileNotFoundException {
return new OutputStreamSampler(new FileOutputStream(file));
Copy link
Member

Choose a reason for hiding this comment

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

This means the stream is open during the whole sampling rather than being opened and close just for the time of stoping and writing the data down.

Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

Some code cleanup notes added. Functionality seems to be fine in spite of them, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backend self-sampling
4 participants