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

Add support for running YourKit on Windows #409

Merged
merged 3 commits into from Mar 8, 2022
Merged

Conversation

sambsnyd
Copy link
Contributor

@sambsnyd sambsnyd commented Mar 3, 2022

No description provided.

Signed-off-by: Sam Snyder <sam@moderne.io>
@@ -35,6 +37,12 @@ public static File findControllerJar() {

public static File findJniLib() {
File yourKitHome = findYourKitHome();
if(OperatingSystem.isWindows()) {
if("32".equals(System.getProperty("sun.arch.data.model"))) {
Copy link
Member

@asodja asodja Mar 4, 2022

Choose a reason for hiding this comment

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

Can we use "os.arch" instead of "sun.arch.data.model"? "sun.arch.data.model" seems to be tied to sun so I am not sure if all JVMs report that property.

I think having a method like OperatingSystem.isLinuxX86() but for windows would make sense.

Copy link
Member

Choose a reason for hiding this comment

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

On second thought, I don't think we need to support 32bit.

Copy link
Member

Choose a reason for hiding this comment

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

@sambsnyd Or you maybe need support for 32bit?

@asodja asodja changed the title Add support for running yourkit on Windows Add support for running YourKit on Windows Mar 4, 2022
@asodja
Copy link
Member

asodja commented Mar 8, 2022

Verified on Windows 64bit.

@asodja asodja merged commit 42d243b into gradle:master Mar 8, 2022
@asodja
Copy link
Member

asodja commented Mar 8, 2022

Thank you for your contribution! I merged it, if you need any change, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants