-
Notifications
You must be signed in to change notification settings - Fork 156
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
Improve Maven benchmarking and add profiling support, take 2 #576
Changes from all commits
3b7a406
a36c1d1
d50591b
0646f5c
f604495
3b425f3
66e9cd7
a450c2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package org.gradle.profiler; | ||
|
||
import javax.annotation.Nullable; | ||
import javax.annotation.OverridingMethodsMustInvokeSuper; | ||
import java.io.File; | ||
import java.io.PrintStream; | ||
import java.util.List; | ||
|
@@ -33,9 +34,14 @@ protected void printDetail(PrintStream out) { | |
out.println(" Targets: " + getTargets()); | ||
} | ||
|
||
public String getExecutablePath() { | ||
public String getExecutablePath(File projectDir) { | ||
String toolHomePath = getToolHome() == null ? System.getenv(getToolHomeEnvName()) : getToolHome().getAbsolutePath(); | ||
return toolHomePath == null ? getExecutableName() : toolHomePath + "/bin/" + getExecutableName(); | ||
return toolHomePath == null ? getExecutablePathWithoutToolHome(projectDir) : toolHomePath + "/bin/" + getExecutableName(); | ||
} | ||
|
||
@OverridingMethodsMustInvokeSuper | ||
protected String getExecutablePathWithoutToolHome(File projectDir) { | ||
return getExecutableName(); | ||
} | ||
|
||
@Override | ||
|
@@ -53,7 +59,7 @@ public File getToolHome() { | |
|
||
@Override | ||
public boolean createsMultipleProcesses() { | ||
return false; | ||
return true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ What does this change do, does it have any consequence? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This informs the profiler invocation part whether to expect a long running daemon or many small processes. It was not used for anything other than Gradle. We now say that other build tools will always start a new process per invocation. |
||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,15 +18,25 @@ | |
import java.io.File; | ||
import java.util.function.Consumer; | ||
|
||
public class Profiler { | ||
public abstract class Profiler { | ||
|
||
public static final Profiler NONE = new Profiler() { | ||
@Override | ||
public boolean requiresGradle() { | ||
return false; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "none"; | ||
} | ||
}; | ||
|
||
/** | ||
* Whether this profiler supports only Gradle builds. | ||
*/ | ||
public abstract boolean requiresGradle(); | ||
Comment on lines
+35
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment on the method is:
Should we rename a method to: |
||
|
||
public void validate(ScenarioSettings settings, Consumer<String> reporter) { | ||
} | ||
|
||
|
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.
Nice 👍