RFR: 8304006: jlink should create the jimage file in the native endian for the target platform [v8]
Jaikiran Pai
jpai at openjdk.org
Sat Mar 18 13:31:22 UTC 2023
On Tue, 14 Mar 2023 08:30:11 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 953:
>>
>>> 951: // and supported for creating an image through jlink. Else returns null.
>>> 952: private static ByteOrder getNativeEndianOfTargetPlatform(String targetPlatform) {
>>> 953: int index = targetPlatform.indexOf("-"); // of the form <operating system>-<arch>
>>
>> `Platform::parsePlatform` is the utility method to parse `ModuleTarget`. It can be updated to include additional architectures.
>
>> `Platform::parsePlatform` is the utility method to parse `ModuleTarget`. It can be updated to include additional architectures.
>
> Alternatively, don't parse it. If we go with Jim's suggestion of a resource file then it is just a simple mapping of the ModuleTarget value, e.g the entry for macox-x64 would be:
>
> macos-amd64.endianness = little
Hello Mandy, Alan, Jim,
I've updated this PR to take into account these suggestions. I went ahead with what Mandy suggested and enhanced the existing (internal) `jdk.tools.jlink.internal.Platform` `record`to additional parse these architectures. It was anyway doing it for a select few.
In addition to that, I also moved the architecture to endianness mapping into this `Platform` class itself. I followed Jim's suggestion to move the mapping out of the code. The `Platform` code now reads this from a `endianness.properties` file which is an internal resource of the `jdk.jlink` module. However, I'm having second thoughts about this part. I'm guessing that when Jim suggested moving this to a resource, it was probably because the code resided within the `JLinkTask` class. Now that I've moved this to an existing `Platform` class which is solely meant for things like these, I feel that we could probably just hard code these architecture to endianness mapping within this class itself, instead of reading it from a properties file. I'm looking for some inputs on what you all think would be the best way to proceed.
Alan, as for the `osname-arch=little/big` vs `arch=little/big` mapping style, I decided to use the `arch=little/big` and let the Platform class parse the architecture out of the string in the `ModuleTarget`. I did it for a couple of reasons:
1. The `Platform` class already has the necessary logic to do that, plus it has to do that anyway (for us to implement the suggestion that Mandy noted about using this in `DefaultImageBuilder`)
2. As far as I could see, the OS name shouldn't play any role in the endianness, so leaving that out felt easier to deal with since we wouldn't have to think of what OS name, arch combinations would be valid.
If you or others feel that it's better to stick with the `osname-arch=little/big` approach, instead of this one, please do let me know and I'll go ahead and do that change.
-------------
PR: https://git.openjdk.org/jdk/pull/11943
More information about the core-libs-dev
mailing list