spark icon indicating copy to clipboard operation
spark copied to clipboard

Expose a profiler API

Open Matyrobbrt opened this issue 3 years ago • 5 comments

This PR exposes a profiler API in spark-api.

Changes made

  • Added a new subproject, spark-proto which all other projects depend on (including the API). This allows the API to access the java object of reports
  • Added a new subproject, api-test, which is a test Forge mod for testing the API.
  • Different classes in the me.lucko.spark.common.sampler package have been moved in the API, some of them as interfaces with hidden implementations using Spark
  • SamplerModule has been changed to use the implementation of the API

Maven setup

As configured in spark-api/build.gradle and spark-proto/build.gradle, I recommend the following maven structure:

  • base group me.lucko.spark
  • Proto in me.lucko.spark:spark-proto shading the proto library
  • API in me.lucko.spark:spark-api, depending on me.lucko.spark:spark-proto

API Usage

Example API usage:

private static void profile() throws Exception {
        final Spark api = SparkProvider.get();
        final Profiler profiler = api.profiler();

        // Infinite report
        {
            final Profiler.Sampler sampler = profiler.createSampler(ProfilerConfiguration.builder()
                    .dumper(ThreadDumper.ALL)
                    .grouper(ThreadGrouper.AS_ONE)
                    .ignoreNative()
                    .build(), (e, msg) -> System.err.println("Error creating sampler: " + e + ": " + msg));
            if (sampler == null)
                throw new AssertionError();

            sampler.start(); // Start the profiler
            Thread.sleep(1000 * 60 * 5); // Wait 5 minutes
            sampler.stop(); // Stop the profiler

            final ProfilerReport report = sampler.dumpReport(ReportConfiguration.builder()
                    .comment("My comment")
                    .sender("Me", UUID.randomUUID())
                    .build());
            System.out.println("Report 1 uploaded to " + report.upload());
            report.saveToFile(Path.of("report1.sparkprofile")); // Save the report
        }

        // Report with set time
        {
            final Profiler.Sampler sampler = profiler.createSampler(ProfilerConfiguration.builder()
                    .dumper(new SpecificThreadDumper(Thread.currentThread()))
                    .grouper(ThreadGrouper.BY_NAME)
                    .ignoreSleeping()
                    .samplingInterval(12)
                    .forceJavaSampler()
                    .duration(Duration.ofMinutes(10))
                    .build(), (e, msg) -> System.err.println("Error creating sampler: " + e + ": " + msg));
            if (sampler == null)
                throw new AssertionError();

            sampler.start(); // Start the profiler
            sampler.onCompleted(ReportConfiguration.builder()
                    .separateParentCalls(true).build())
                    .whenComplete((report, throwable) -> {
                        if (throwable != null)
                            throw new RuntimeException(throwable);
                        System.out.println("Report 2 uploaded to " + report.upload());
                    }); // Configure the dump after 10 minutes, when the sampler is done
        }
}

Effectively supersedes #131.

Matyrobbrt avatar Jul 17 '22 16:07 Matyrobbrt

+1. I would like to use Spark to profile the integrated server startup (i.e. before I can enter a command) and this would be very useful.

embeddedt avatar Jul 17 '22 23:07 embeddedt

The PR is ready for review.

Matyrobbrt avatar Aug 09 '22 09:08 Matyrobbrt

Hi, thanks for the PR! I will try to have a deeper look at this soon.

I will be totally honest/upfront with you, I have a few reservations about merging this currently:

  • Systems that automatically trigger the profiler and upload the data are already causing a problem for me in terms of data storage (remember that spark data is uploaded centrally). I am concerned that this is only going to get worse if we add an API for it. I would at least like to get that under control (it is barely at the moment) & fully understand the $ costs before we add a feature to accelerate the data usage 😛
  • I would prefer to not have implementation classes in the API package/module. It should contain only interfaces and enums ideally.
  • If the API is tightly coupled to the implementation (as it seems to be at the moment), it makes it more difficult to make changes to spark-common and the platforms in the future, which may hinder improvements or new functionality down the line.

That's where I'm at atm :) please feel free to ping/chat to me on Discord (https://discord.gg/PAGT2fu) if you have any comments or suggestions, or of course you can reply here.

lucko avatar Aug 29 '22 08:08 lucko

Moved discussion to Discord

Matyrobbrt avatar Sep 05 '22 05:09 Matyrobbrt

hey, i'm curious if this is still planned to be added or has been cancelled

ArDiDEx avatar May 08 '23 18:05 ArDiDEx