-
-
Notifications
You must be signed in to change notification settings - Fork 180
Expose a profiler API #229
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
Conversation
embeddedt
commented
Jul 17, 2022
+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.
Matyrobbrt
commented
Aug 9, 2022
The PR is ready for review.
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.
Matyrobbrt
commented
Sep 5, 2022
Moved discussion to Discord
ArDiDEx
commented
May 8, 2023
hey, i'm curious if this is still planned to be added or has been cancelled
Uh oh!
There was an error while loading. Please reload this page.
This PR exposes a profiler API in
spark-api.Changes made
spark-protowhich all other projects depend on (including the API). This allows the API to access the java object of reportsapi-test, which is a test Forge mod for testing the API.me.lucko.spark.common.samplerpackage have been moved in the API, some of them as interfaces with hidden implementations usingSparkSamplerModulehas been changed to use the implementation of the APIMaven setup
As configured in
spark-api/build.gradleandspark-proto/build.gradle, I recommend the following maven structure:me.lucko.sparkme.lucko.spark:spark-protoshading the proto libraryme.lucko.spark:spark-api, depending onme.lucko.spark:spark-protoAPI Usage
Example API usage:
Effectively supersedes #131.