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

NETBEANS-1428 Fix fatal error when profiling on Windows #2021

Closed
wants to merge 4 commits into from

Conversation

pedro-w
Copy link
Contributor

@pedro-w pedro-w commented Mar 16, 2020

This PR includes the following:

  • Update the native code for 64-bit (all platforms) which fixes the fatal error on Windows and also makes the code more correct for Linux, macOS
  • Update the native code build scripts for Windows, Linux and macOS
  • Update the lib.profiler process including the download of the binary archive of native libraries

No changes were necessary to the Java code.
Please see NETBEANS-1428 for discussion.

NOTE The binary archive has not yet been uploaded, so simply merging this PR for testing purposes will not work 'out of the box'.

* Stacks.c: corrected some incorrect casts for 64-bit windows. Implemented
  mapping from methodIds to ints using a hash table for 64-bit platforms.
* Updated build scripts for Windows, Linux and Mac.
* Moved scripts out of the build subdirectory.
@ebarboni
Copy link
Contributor

I cannot review the native code, but maybe we should make a dedicated repo to build all the native element with their source, and only use the resulting lib in the main netbeans repo

@pedro-w
Copy link
Contributor Author

pedro-w commented Mar 16, 2020

It is tricky because the native code needs to be built on each platform individually and then all the binary libraries combined into a zip. I've done this for Windows, macOS and Linux so if there is a suitable place I can upload it there and people can then download it manually to test.

@ebarboni
Copy link
Contributor

I thinks the launcher executable are build on linux but that build exe for windows? if this is possible for mac we could maybe use a jenkins to prepare them based on sources identified in a repo

@pedro-w
Copy link
Contributor Author

pedro-w commented Mar 16, 2020

That would be better than the current if it is possible. Who would know the answer/how to do it? I can ask them.

@matthiasblaesing
Copy link
Contributor

If I remember correctly the building of the launcher binaries was refactored in the apache netbeans timeframe, so git blame on the relevant files should indicate, who might be able to help. If that does not work, take it to the dev@ mailing list.

@pedro-w
Copy link
Contributor Author

pedro-w commented Mar 26, 2020

If I remember correctly the building of the launcher binaries was refactored in the apache netbeans timeframe, so git blame on the relevant files should indicate, who might be able to help. If that does not work, take it to the dev@ mailing list.

It might be @lkishalmi ?

@eirikbakke
Copy link
Contributor

I did an "Attach to External Process" profiler session with Windows 10/NetBeans 12.1/OpenJDK 11.0.9 today, and ran into this problem again, with the JVM crashing. Swapping in Peter Hull's profilerinterface.dll file from two years ago, linked from the JIRA issue comment, once again fixed the problem (thanks, Peter!). Would be great to have this merged.

(Swamped with other work at the moment, otherwise I'd contribute more here...)

@pedro-w
Copy link
Contributor Author

pedro-w commented Nov 21, 2020

I don't use Netbeans or Java all that much these days but it would be good to get this finished off. I had the binaries all ready to go but no-one would tell me how I could get them uploaded to the OSUOSL server. Hopefully I can find them again!
Another aspect of this which is new since March - a whole new platform (Apple Silicon) has appeared and I have no way to develop or test it.

@eirikbakke
Copy link
Contributor

@pedro-w Yeah, the precompiled binaries aspect is the only reason why I don't know the proper way to merge this myself...

Another aspect of this which is new since March - a whole new platform (Apple Silicon) has appeared and I have no way to develop or test it.

I think nobody does at the moment :-) First there needs to be an OpenJDK release for Apple Silicon, which Azul and Microsoft(!) are currently working on. And slowly but surely more NetBeans developers will be buying the new MacBooks. Until then I expect NetBeans will be running on new MacBooks under the Rosetta 2 translation layer.

@errael
Copy link
Contributor

errael commented Nov 21, 2020

@pedro-w Yeah, the precompiled binaries aspect is the only reason why I don't know the proper way to merge this myself...

Who would know? Could they be added as participants?

@eirikbakke
Copy link
Contributor

Perhaps @jlahoda or @sdedic might know about the process for uploading new binaries.

@jlahoda
Copy link
Contributor

jlahoda commented Nov 24, 2020

Regarding uploading of the binaries, I believe this is what could be used:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115507251

@lkishalmi
Copy link
Contributor

I can do uploads.
Also in the future it would be better to compile these binaries during build.
Maybe using cross compiler for Windows and Mac. Has anyone an idea how to do that?

@pedro-w
Copy link
Contributor Author

pedro-w commented Nov 24, 2020

Has anyone an idea how to do that?

I don't know but it may be possible to have a github action to build the native code and then make it available.
https://github.community/t/caching-files-between-github-action-executions/16047/5
That would cover Windows, some form of Linux and some form of MacOS but it's not really under our control.
I believe it's possible to cross compile to Windows on Linux (with mingw32) and vice versa (with WSL?) but neither can produce Mac binaries.
Otherwise, does Apache have some kind of resource for doing this?

@pedro-w
Copy link
Contributor Author

pedro-w commented Nov 24, 2020

I've put the binaries
https://github.com/pedro-w/netbeans/releases/tag/v2
When I built them I was sure I had the correct zip file layout and file name to upload to the server, that was March so I am a bit hazy on the details now.

edit: Still seems to work, see https://issues.apache.org/jira/browse/NETBEANS-1428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17240450#comment-17240450

@errael
Copy link
Contributor

errael commented Jan 17, 2021

12.3? @geertjanw @neilcsmith-net

There's been some vigorous discussion in dev thread, mid january 2021, thread subject Profiling on 64bit Windows. There's a lot about the "right" way to do it.

Most import for a near term solution is Jan's analysis.

I don't think it would be so bad. The zip with the binaries is unpacked by
the build process, and (AFAIK) only this unpacked version is used in the
product. It would be trivial to have another AL2-licensed zip, which would
be unpacked over the unpacked CDDL licensed zip, overwriting same-named
files.

and

Overall, my suggestion would be to create a new zip for the AL2-licensed
profiler binaries, and unpack it over the unpacked content of the existing
CDDL-licensed archive. Should be reasonably easy to setup.

Poking around, in repo, there's netbeans/profiler/lib.profiler/
In it's build.xml there's:

<target name="-process.release.files">
    <unzip src="external/profiler-external-binaries-8.2.zip" dest="${profiler.cluster}"/>
</target>

I have confirmed that stuff from

https://github.com/pedro-w/netbeans/releases/tag/v0.1-alpha
profilerinterface-jdk16-windows-amd64.zip

Has files with names that match stuff under this directory and what is seen in a release.

Seems so close...

@neilcsmith-net
Copy link
Member

@errael not an RM's job to specifically decide whether this gets in or not. Although if it gets in and somehow made the build unreleasable, that would be a different matter. Personally I'm not sure just using the binaries from there is a good idea - maybe built and signed locally by a PMC / committer is a short-term option, then put on OSUOSL?

@errael
Copy link
Contributor

errael commented Jan 17, 2021

@neilcsmith-net right.

I guess the optimist in me is hoping that with broad awareness and increasing comments/information/opinions, this "trivial" (to some) task/process will be handled.

@pedro-w
Copy link
Contributor Author

pedro-w commented Jan 21, 2021

Closed in favour of #2700

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

Successfully merging this pull request may close these issues.

None yet

8 participants