-
Notifications
You must be signed in to change notification settings - Fork 153
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
Use absolute paths for async-profiler output files #383
Conversation
When testing Android Studio sync with gradle-profiler I noticed after a long debugging session that profiler does not work if absolute --output-dir is not set. This fixes it so absolute paths are passed in JVM args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably fixes the absolute path problem for async profiler. Are other profilers affected as well?
@@ -34,7 +34,7 @@ public static AsyncProfilerOutputType from(AsyncProfilerConfig config, GradleSce | |||
abstract File outputFileFor(ScenarioSettings settings); | |||
|
|||
File individualOutputFileFor(ScenarioSettings settings) { | |||
File outputFile = outputFileFor(settings); | |||
File outputFile = outputFileFor(settings).getAbsoluteFile(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better if outputFileFor
would return an absolute file already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I also tested jfr, YourKit, JProfiler and they don't have this issue.
@@ -9,13 +9,13 @@ | |||
STACKS("collapsed") { | |||
@Override | |||
File outputFileFor(ScenarioSettings settings) { | |||
return settings.profilerOutputLocationFor(".stacks.txt"); | |||
return settings.profilerOutputLocationFor(".stacks.txt").getAbsoluteFile(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should go even deeper: It seems like ScenarioDefinition.getOutputDir()
or ScenarioDefinition.outputDir
should always be an absolute file. Then the rest would follow automatically I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
When testing Android Studio sync with gradle-profiler I noticed after a long debugging session that profiler does not work if absolute --output-dir is not set. This fixes it so absolute paths are passed in JVM args.