On Thu, 22 Aug 2024 22:49:50 GMT, David Holmes <dhol...@openjdk.org> wrote:
> is the final intent here that this one magic file will be compiled first with > an inline declaration such that when the other files containing the apparent > runtime check get compiled, it can actually be determined at build time and > so have the same effects as the old ifdef logic? Theoretically, this is a valid complaint: what is now inlined at compilation time will require an additional function call. And yes, if that had been a performance issue, I would have needed to do something like that. But, if you look at the actual functions that are affected, you can see that it is just a handful of calls that are all done at startup time. Adding like half a dozen calls to a trivial function before loading a DLL will not even be measurable, compared to all the work the OS will do afterwards when loading the DLL. So no, I do not intend to complicate the code further. Any impact of this code is measured in a few additional machine code operations. > There are also other source-level solutions possible here by refactoring the > code that has static vs dynamic linking dependencies into its own files and > the build system can then select which set of files to compile. There are definitely refactoring/restructuring opportunities to be had, both in Hotspot and in the JDK libraries! Overall, I have found a lot of redundant work, duplicated code, and legacy code that does not make sense anymore (like trying to differentiate between a JRE and a JDK) when setting up the initial environment wrt the basic dynamic libraries. But in the grand scheme of things, I don't think it is reasonable that we spend too much efforts on cleaning up that. While it is definitely a "lava flow" anti-pattern, it mostly works, and starting to poke around will risk breaking things. We don't have a good testing story for the JDK bootstrapping. This is the same problem as the build is facing: you would need to have a ton of differently configured environments to be able to test all possible installations and paths etc. The patch presented here seem to me to be a cautious middle ground -- fixing what is needed to be able to progress, but doing so in a way that every single change is trivially and obviously correct. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20666#issuecomment-2306752185
