On Thu, 22 Feb 2024 07:44:48 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> **Summary:** Finally get rid of the mapfiles in Hotspot, and replace it with >> compiler options and `JNIEXPORT` on all platforms. >> >> The bug that this PR solves, >> [JDK-8017234](https://bugs.openjdk.org/browse/JDK-8017234), was created in >> 2013. Even back then the use of mapfiles in Hotspot was dated, so this is >> really good riddance with old rubbish. >> >> This code touches on central but not well understood parts of the Hotspot >> dynamic library, which has contributed to why this bug has stayed unresolved >> for so long. I will need to explain this fix in more detail than usually >> necessary. (Please bare with me if this gets long.) I also anticipate that >> not all solutions that I've picked will be accepted, and we'll have to >> discuss how to proceed. I think it is better to have actual concrete code to >> discuss around, rather than starting by an abstract discussion. To keep this >> description short, I will post the discussion as a comment to the PR. >> >> I have run this PR through tier 1-3 in our CI system. I have also carefully >> checked how the resulting dynamic library differs with this patch (not much; >> see discussion below). For build system changes, this is often the most >> relevant metric. > > src/hotspot/share/jvmci/jvmciCompilerToVM.cpp line 74: > >> 72: >> 73: #if defined(TARGET_COMPILER_gcc) >> 74: #undef JNIEXPORT > > Based on the suggestion in [this JBS > comment](https://bugs.openjdk.org/browse/JDK-8017234?focusedId=14601421&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14601421), > I think we can remove JNIEXPORT from all the functions in this file. > > We need to keep the JNICALL though; the deprecated Win32 port needs it, not > sure if it's used anywhere else. I think so too. However, I would *really* like to do that separately as a follow up. My goal with this PR has been to make no changes in the product. Consider what would happen if we removed this, and there was some strange failure down the line. There is a high risk the entire PR would be backed out. If instead we remove the JNIEXPORT in a separate change, and this causes failures, then it is a much easier backout, and we would be much better informed at exactly what failed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1498956646
