Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers
Adding 2d-dev in the hopes of getting some input from component team. Seems like they should be aware of us removing the support for multiple data models. Looks like you left a debug message at line 40 in GensrcX11Wrappers.gmk. /Erik On 2018-03-02 03:00, Magnus Ihse Bursie wrote: On 2018-03-02 00:02, Erik Joelsson wrote: Hello, In xlibtypes.txt comment, should it be sizes-64.txt? Yes, good catch. Generating both 32 and 64 seems a bit outdated at this point. Surely this is a remnant of bundling 64 and 32 bit together on Solaris in the past? Perhaps someone in 2d can answer this? Would be nice to be able to clean up that part as well if possible. Yes, you are right. We should clean up that as well. I was just too conservative. I've now actually checked what happens when you generate both 32 and 64 bit versions, and the result is that instead of code like: public static int getSize() { return 96; } we get code like this: public static int getSize() { return ((XlibWrapper.dataModel == 32)?(80):(96)); } Since we do no longer support multiple data models for the same build, this is just unnecessary. In fact, that leads to an even better cleanup, since we will always need just a single input file. I also wrapped the tool calls in ExecuteWithLog. Updated webrev: http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.02 /Magnus
Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers
On 2018-03-02 00:02, Erik Joelsson wrote: Hello, In xlibtypes.txt comment, should it be sizes-64.txt? Yes, good catch. Generating both 32 and 64 seems a bit outdated at this point. Surely this is a remnant of bundling 64 and 32 bit together on Solaris in the past? Perhaps someone in 2d can answer this? Would be nice to be able to clean up that part as well if possible. Yes, you are right. We should clean up that as well. I was just too conservative. I've now actually checked what happens when you generate both 32 and 64 bit versions, and the result is that instead of code like: public static int getSize() { return 96; } we get code like this: public static int getSize() { return ((XlibWrapper.dataModel == 32)?(80):(96)); } Since we do no longer support multiple data models for the same build, this is just unnecessary. In fact, that leads to an even better cleanup, since we will always need just a single input file. I also wrapped the tool calls in ExecuteWithLog. Updated webrev: http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.02 /Magnus
Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers
Hello, In xlibtypes.txt comment, should it be sizes-64.txt? Generating both 32 and 64 seems a bit outdated at this point. Surely this is a remnant of bundling 64 and 32 bit together on Solaris in the past? Perhaps someone in 2d can answer this? Would be nice to be able to clean up that part as well if possible. Otherwise good. /Erik On 2018-02-28 12:12, Magnus Ihse Bursie wrote: My hunt on technical debt continues. This time my aim has been on the sorry mess known as GensrcX11Wrappers. I've disentangled it into two functions, one simple gensrc part that is actually run during the build, and which involves just a simple java tool and some pre-calculated data files, and a separate step for updating those pre-calculated data files. This step is now run using "make update-x11wrappers". It involves compiling a native binary, and running it on the target platform, so just as before, this assumes that you are not cross-compiling. I'm not sure what role the "verification" step we had before ever played. For all the years we've been "verifying" this, we've detected no differences. In fact, as far as I understand, what we *really* would need to verify against, is the X11 libraries on the runtime system, i.e. where the users executes the JRE. But then again, we can assume that this matches, just as anyone compiling with header files on one place can assume that they can use the libraries elsewhere. The only reason, as I see it, to keep the generation in the makefiles at all, is to document how the files were generated, and to be ready for the need to update the files if the list of datatypes in xlibtypes.txt changes. Bug: https://bugs.openjdk.java.net/browse/JDK-8198844 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.01 /Magnus