Re: RFR: JDK-8242943 Fix all remaining unchecked warnings in jdk.hotspot.agent
Hi Magnus,
That all seems reasonable to me.
Thanks,
David
-
On 16/04/2020 10:47 pm, Magnus Ihse Bursie wrote:
This is the final part of removing all warnings from the build of
jdk.hotspot.agent. This patch includes a number of non-trivial fixes for
the few remaining unchecked warnings.
The good news is that with this fix (and after the recent removal of
Nashorn and rmic), the JDK build is finally complete free from warnings
for the first time ever! EPIC!1!!! :-)
The bad news is that this patch is probably the most complex of all I've
posted so far. :-/
I'll break down the changes in four groups:
* ConstMethod superclass issue
ConstMethod current extends VMObject. However, it is treated as a
Metadata (Metadata is a subtype of VMObject). For instance, this code
appears in Metadata.java:
metadataConstructor.addMapping("ConstMethod", ConstMethod.class);
I believe it is just an oversight that ConstMethod does not also extend
Metadata like the rest of the classes added to the metadataConstructor
mapping, one which has not been caught without the extra type safety
given by generics.
* ProfileData casting
In several places, a ProfileData item is extracted from a collection,
its type checked with instanceof, and then acted on accordingly. (Yeah,
nice and clean OOP abstraction at work! :-))
This works well most of the time, but when casting to ReceiverTypeData
or CallTypeDataInterface, the type include generic parametrisation, so
it needs to be cast to e.g. ReceiverTypeData.
Unfortunately, this cannot be verified using instanceof, due to generic
type erasure, and thus the compiler considers this an unchecked cast.
(At least, this is my understanding of what is happening.) As far as I
can tell, this is legitimate code, and the only way to really handle
this (barring a more extensive rewrite) is to disable the warning for
these cases.
* Generification of the InstanceConstructor hierarchy
I have properly generified the InstanceConstructor hierarchy, with the
VirtualConstructor, StaticBaseConstructor and VirtualBaseConstructor
subclasses. (And also the related helper class VMObjectFactory.)
Most of it turned out OK (and with improved type safety), but there was
one piece of code that is a bit unfortunate. In VirtualBaseConstructor,
we have the following:
@SuppressWarnings("unchecked")
Class lookedUpClass = (ClassT>)Class.forName(packageName + "." + superType.getName());
c = lookedUpClass;
That is, we send in a name for a class and assumes that it will resolve
to a subtype of T, but there is no way to enforce this with type safety.
I have verified all current callers to the constructor so that they all
abide to this rule (of course, otherwise the code would not have worked,
but I checked it anyway). The alternative to suppressing the warning is
to redesign the entire API, so we do not send in a class name, but
instead a Class, and use that to extract the name instead.
At this point, I don't find it worth it. This is, after all, no worse
than it was before.
* SortableTableModel and TableModelComparator
This one was quite messy. The tables are being used in quite different
ways in different places, which made it hard to generify in a good way.
A lot of it revolves around keeping the data as Objects and casting it
to a known type. (Some of this behavior comes from the fact that they
extend old Swing classes which does this.) I've tried to tighten it up
as much as possible by introducing increased type safety by
generification, but in the end, it was not fully possible to do without
a major surgery of the entire class structure. As above, most of it
turned out OK, but not all.
The tricky one here is the helper TableModelComparator. This one took me
a while to wrap my head around. It implements Comparator, but the
compare() method takes "rows" from the model, which are just expressed
as Objects, and left to subclasses to define differently. For one of the
subclasses it uses the type T that the TableModel is created around, but
in other it uses an independent domain model. :-/ Anyway. The compare()
method then extracts data for the individual columns of each row using
the method getValueForColumn(), and compare them pairwise. This data
from the rows are supposed to implement Comparable.
In the end, I think I got it pretty OK, but I'm still an apprentice when
it comes to generics black magic. The subclasses of TableModelComparator
want to return different objects from getValueForColumn() for different
columns in the row, like Long or String. They are all Comparable, but
String is Comparable and Long is Comparable. So I needed
to specify the abstract method as returning Comparable, since
Comparable is not Comparable.
But then, for reasons that I do not fully fathom, I could not specify
Comparable o1 = getValueForColumn(row1, columns[i]);
Comparable o2 = getValueForColumn(row2, columns[i])
Re: RFR: JDK-8242943 Fix all remaining unchecked warnings in jdk.hotspot.agent
Hi Magnus,
This looks pretty good to me.
I hope, Joe gave some insight for Comparable's.
Thanks,
Serguei
On 4/16/20 05:47, Magnus Ihse Bursie wrote:
This is the final part of removing all warnings from the build of
jdk.hotspot.agent. This patch includes a number of non-trivial fixes
for the few remaining unchecked warnings.
The good news is that with this fix (and after the recent removal of
Nashorn and rmic), the JDK build is finally complete free from
warnings for the first time ever! EPIC!1!!! :-)
The bad news is that this patch is probably the most complex of all
I've posted so far. :-/
I'll break down the changes in four groups:
* ConstMethod superclass issue
ConstMethod current extends VMObject. However, it is treated as a
Metadata (Metadata is a subtype of VMObject). For instance, this code
appears in Metadata.java:
metadataConstructor.addMapping("ConstMethod", ConstMethod.class);
I believe it is just an oversight that ConstMethod does not also
extend Metadata like the rest of the classes added to the
metadataConstructor mapping, one which has not been caught without the
extra type safety given by generics.
* ProfileData casting
In several places, a ProfileData item is extracted from a collection,
its type checked with instanceof, and then acted on accordingly.
(Yeah, nice and clean OOP abstraction at work! :-))
This works well most of the time, but when casting to ReceiverTypeData
or CallTypeDataInterface, the type include generic parametrisation, so
it needs to be cast to e.g. ReceiverTypeData.
Unfortunately, this cannot be verified using instanceof, due to
generic type erasure, and thus the compiler considers this an
unchecked cast. (At least, this is my understanding of what is
happening.) As far as I can tell, this is legitimate code, and the
only way to really handle this (barring a more extensive rewrite) is
to disable the warning for these cases.
* Generification of the InstanceConstructor hierarchy
I have properly generified the InstanceConstructor hierarchy, with the
VirtualConstructor, StaticBaseConstructor and VirtualBaseConstructor
subclasses. (And also the related helper class VMObjectFactory.)
Most of it turned out OK (and with improved type safety), but there
was one piece of code that is a bit unfortunate. In
VirtualBaseConstructor, we have the following:
@SuppressWarnings("unchecked")
Class lookedUpClass = (ClassT>)Class.forName(packageName + "." + superType.getName());
c = lookedUpClass;
That is, we send in a name for a class and assumes that it will
resolve to a subtype of T, but there is no way to enforce this with
type safety. I have verified all current callers to the constructor so
that they all abide to this rule (of course, otherwise the code would
not have worked, but I checked it anyway). The alternative to
suppressing the warning is to redesign the entire API, so we do not
send in a class name, but instead a Class, and use that
to extract the name instead. At this point, I don't find it worth it.
This is, after all, no worse than it was before.
* SortableTableModel and TableModelComparator
This one was quite messy. The tables are being used in quite different
ways in different places, which made it hard to generify in a good
way. A lot of it revolves around keeping the data as Objects and
casting it to a known type. (Some of this behavior comes from the fact
that they extend old Swing classes which does this.) I've tried to
tighten it up as much as possible by introducing increased type safety
by generification, but in the end, it was not fully possible to do
without a major surgery of the entire class structure. As above, most
of it turned out OK, but not all.
The tricky one here is the helper TableModelComparator. This one took
me a while to wrap my head around. It implements Comparator, but the
compare() method takes "rows" from the model, which are just expressed
as Objects, and left to subclasses to define differently. For one of
the subclasses it uses the type T that the TableModel is created
around, but in other it uses an independent domain model. :-/ Anyway.
The compare() method then extracts data for the individual columns of
each row using the method getValueForColumn(), and compare them
pairwise. This data from the rows are supposed to implement Comparable.
In the end, I think I got it pretty OK, but I'm still an apprentice
when it comes to generics black magic. The subclasses of
TableModelComparator want to return different objects from
getValueForColumn() for different columns in the row, like Long or
String. They are all Comparable, but String is Comparable and
Long is Comparable. So I needed to specify the abstract method
as returning Comparable, since Comparable is not
Comparable.
But then, for reasons that I do not fully fathom, I could not specify
Comparable o1 = getValueForColumn(row1, columns[i]);
Comparable o2 = getV
RFR: JDK-8243109: Bootcycle build failures after Nashorn removal
With Nashorn removed, the bootcycle build is now broken for builds that process markdown files for docs or man pages. Same goes for building with with a recent enough JDK 15 as your bootjdk. While a long term solution is being worked on, we need something to keep our CI builds from failing. For now that would simply be disabling these parts of the build if a jjs launcher cannot be found in the boot jdk. This patch checks the bootjdk for jjs. If not found, ENABLE_PANDOC is set to false. This will in turn prevent full docs from being enabled. For local bootcycle builds, I explicitly disable pandoc in bootcycle-spec.gmk.in. For the jib profile linux-x64-bootcycle-prebuilt, the "--enable-full-docs" configure arg is inherited from the main profile, so needed to neutralize that with an override. I had to reorder a little bit in configure.ac to have the bootjdk setup when processing JDK options. Webrev: http://cr.openjdk.java.net/~erikj/8243109/webrev.01/index.html Bug: https://bugs.openjdk.java.net/browse/JDK-8243109 /Erik
Re: cross-compile JDK 14 with arm-linux-gnueabihf target
Hello Jiwon, Are you able to build a native JDK for the x86 platform you are running the build on or is that giving the same errors? Since JDK 9, when cross compiling, we need a native JDK to run some of the build steps with (jmod and jlink mainly). This native JDK (referred to as the "build" jdk in configure and the makefiles) has to match the version you are cross compiling exactly. If one is not provided, the default is to build one on the fly using the sources present, but we only compile the modules necessary to run the build steps. A way to work around this is to explicitly build a native JDK from the same sources first, and then provide that image to the cross compilation build using --with-build-jdk=... That may help you here, not sure. It may at least be easier to troubleshoot the native build if done explicitly. Especially if you need to disable warnings in that build as well. I don't think you need all of those configure arguments. Configure should find the correct compiler for your target if present on your path. The sysroot setting should get you the correct system include paths etc. If freetype is installed in your sysroot in a reasonable location, it should be found automatically. Disabling warnings should be done using --disable-warnings-as-errors. Linking stdc++ static should be done with --with-stdc++lib=,,. /Erik On 2020-04-20 14:04, Choe, Jiwon wrote: Hello all, I'm trying to cross-compile OpenJDK 14 to target arm-linux-gnueabihf, and the build is failing for me with these errors: === Output from failing command(s) repeated here === * For target buildjdk_hotspot_variant-server_libjvm_objs_os_linux_x86.o: /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp: In static member function 'static u_char* os::Linux::ucontext_get_pc(const ucontext_t*)': /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:120:35: error: 'const mcontext_t' has no member named 'gregs' return (address)uc->uc_mcontext.gregs[REG_PC]; ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:90:16: error: 'REG_EIP' was not declared in this scope #define REG_PC REG_EIP ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:120:41: note: in expansion of macro 'REG_PC' return (address)uc->uc_mcontext.gregs[REG_PC]; ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp: In static member function 'static void os::Linux::ucontext_set_pc(ucontext_t*, address)': /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:124:19: error: 'mcontext_t' has no member named 'gregs' uc->uc_mcontext.gregs[REG_PC] = (intptr_t)pc; ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:90:16: error: 'REG_EIP' was not declared in this scope ... (rest of output omitted) Although my build system is 32-bit x86 Linux, I'm confused because it seems odd that a cross-compile for ARM would need to compile something in a linux_x86 directory. These are the steps I took for the build: 1. sudo qemu-debootstrap --arch=armhf --verbose --include=fakeroot,build-essential,libx11-dev,libxext-dev,libxrender-dev,libxtst-dev,libxt-dev,libcups2-dev,libfontconfig1-dev,libfreetype6-dev,libasound2-dev,libc6-dev,gcc-multilib,g++-multilib --resolve-deps trusty /opt/sysroot/ 2. bash configure --openjdk-target=arm-linux-gnueabihf --with-sysroot=/opt/sysroot/ --with-freetype-include=/opt/sysroot/usr/include/freetype2 --with-freetype-lib=/opt/sysroot/usr/lib/arm-linux-gnueabihf --with-extra-cflags='-Wno-error -I/opt/sysroot/usr/include/arm-linux-gnueabihf/c++/4.8 -I/opt/sysroot/usr/include/c++/4.8' --with-extra-cxxflags='-Wno-error -I/opt/sysroot/usr/include/arm-linux-gnueabihf/c++/4.8 -I/opt/sysroot/usr/include/c++/4.8' --with-stdc++lib=static CC=arm-linux-gnueabihf-gcc CXX=arm-linux-gnueabihf-g++ 3. make images The steps above worked for me when I tried the same cross-compile for JDK 8. I had an additional flag --with-jvm-variants=client in the configure stage for JDK 8. I tried both client and server variants for JDK 14, and I get the same error. If anyone has insight into how to fix or work around this issue, please let me know! Thanks in advance, Jiwon Choe
Re: cross-compile JDK 14 with arm-linux-gnueabihf target
Is the ARM compiler on your path? I use the —with-devkit configure option to point to the compiler. Bob. > On Apr 20, 2020, at 5:04 PM, Choe, Jiwon wrote: > > Hello all, > > I'm trying to cross-compile OpenJDK 14 to target arm-linux-gnueabihf, and > the build is failing for me with these errors: > > === Output from failing command(s) repeated here === > * For target buildjdk_hotspot_variant-server_libjvm_objs_os_linux_x86.o: > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp: In > static member function 'static u_char* os::Linux::ucontext_get_pc(const > ucontext_t*)': > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:120:35: > error: 'const mcontext_t' has no member named 'gregs' > return (address)uc->uc_mcontext.gregs[REG_PC]; > ^ > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:90:16: > error: 'REG_EIP' was not declared in this scope > #define REG_PC REG_EIP >^ > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:120:41: > note: in expansion of macro 'REG_PC' > return (address)uc->uc_mcontext.gregs[REG_PC]; > ^ > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp: In > static member function 'static void os::Linux::ucontext_set_pc(ucontext_t*, > address)': > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:124:19: > error: 'mcontext_t' has no member named 'gregs' > uc->uc_mcontext.gregs[REG_PC] = (intptr_t)pc; > ^ > /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:90:16: > error: 'REG_EIP' was not declared in this scope > ... (rest of output omitted) > > > Although my build system is 32-bit x86 Linux, I'm confused because it seems > odd that a cross-compile for ARM would need to compile something in a > linux_x86 directory. > > > These are the steps I took for the build: > > 1. sudo qemu-debootstrap --arch=armhf --verbose > --include=fakeroot,build-essential,libx11-dev,libxext-dev,libxrender-dev,libxtst-dev,libxt-dev,libcups2-dev,libfontconfig1-dev,libfreetype6-dev,libasound2-dev,libc6-dev,gcc-multilib,g++-multilib > --resolve-deps trusty /opt/sysroot/ > > 2. bash configure --openjdk-target=arm-linux-gnueabihf > --with-sysroot=/opt/sysroot/ > --with-freetype-include=/opt/sysroot/usr/include/freetype2 > --with-freetype-lib=/opt/sysroot/usr/lib/arm-linux-gnueabihf > --with-extra-cflags='-Wno-error > -I/opt/sysroot/usr/include/arm-linux-gnueabihf/c++/4.8 > -I/opt/sysroot/usr/include/c++/4.8' --with-extra-cxxflags='-Wno-error > -I/opt/sysroot/usr/include/arm-linux-gnueabihf/c++/4.8 > -I/opt/sysroot/usr/include/c++/4.8' --with-stdc++lib=static > CC=arm-linux-gnueabihf-gcc CXX=arm-linux-gnueabihf-g++ > > 3. make images > > > The steps above worked for me when I tried the same cross-compile for JDK > 8. I had an additional flag --with-jvm-variants=client in the configure > stage for JDK 8. I tried both client and server variants for JDK 14, and I > get the same error. > > If anyone has insight into how to fix or work around this issue, please let > me know! > > Thanks in advance, > Jiwon Choe
cross-compile JDK 14 with arm-linux-gnueabihf target
Hello all, I'm trying to cross-compile OpenJDK 14 to target arm-linux-gnueabihf, and the build is failing for me with these errors: === Output from failing command(s) repeated here === * For target buildjdk_hotspot_variant-server_libjvm_objs_os_linux_x86.o: /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp: In static member function 'static u_char* os::Linux::ucontext_get_pc(const ucontext_t*)': /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:120:35: error: 'const mcontext_t' has no member named 'gregs' return (address)uc->uc_mcontext.gregs[REG_PC]; ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:90:16: error: 'REG_EIP' was not declared in this scope #define REG_PC REG_EIP ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:120:41: note: in expansion of macro 'REG_PC' return (address)uc->uc_mcontext.gregs[REG_PC]; ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp: In static member function 'static void os::Linux::ucontext_set_pc(ucontext_t*, address)': /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:124:19: error: 'mcontext_t' has no member named 'gregs' uc->uc_mcontext.gregs[REG_PC] = (intptr_t)pc; ^ /home/sim32/jdk14/jdk14/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp:90:16: error: 'REG_EIP' was not declared in this scope ... (rest of output omitted) Although my build system is 32-bit x86 Linux, I'm confused because it seems odd that a cross-compile for ARM would need to compile something in a linux_x86 directory. These are the steps I took for the build: 1. sudo qemu-debootstrap --arch=armhf --verbose --include=fakeroot,build-essential,libx11-dev,libxext-dev,libxrender-dev,libxtst-dev,libxt-dev,libcups2-dev,libfontconfig1-dev,libfreetype6-dev,libasound2-dev,libc6-dev,gcc-multilib,g++-multilib --resolve-deps trusty /opt/sysroot/ 2. bash configure --openjdk-target=arm-linux-gnueabihf --with-sysroot=/opt/sysroot/ --with-freetype-include=/opt/sysroot/usr/include/freetype2 --with-freetype-lib=/opt/sysroot/usr/lib/arm-linux-gnueabihf --with-extra-cflags='-Wno-error -I/opt/sysroot/usr/include/arm-linux-gnueabihf/c++/4.8 -I/opt/sysroot/usr/include/c++/4.8' --with-extra-cxxflags='-Wno-error -I/opt/sysroot/usr/include/arm-linux-gnueabihf/c++/4.8 -I/opt/sysroot/usr/include/c++/4.8' --with-stdc++lib=static CC=arm-linux-gnueabihf-gcc CXX=arm-linux-gnueabihf-g++ 3. make images The steps above worked for me when I tried the same cross-compile for JDK 8. I had an additional flag --with-jvm-variants=client in the configure stage for JDK 8. I tried both client and server variants for JDK 14, and I get the same error. If anyone has insight into how to fix or work around this issue, please let me know! Thanks in advance, Jiwon Choe
Re: RFR: JDK-8242943 Fix all remaining unchecked warnings in jdk.hotspot.agent
Hello, On 4/16/2020 5:47 AM, Magnus Ihse Bursie wrote: [snip] The tricky one here is the helper TableModelComparator. This one took me a while to wrap my head around. It implements Comparator, but the compare() method takes "rows" from the model, which are just expressed as Objects, and left to subclasses to define differently. For one of the subclasses it uses the type T that the TableModel is created around, but in other it uses an independent domain model. :-/ Anyway. The compare() method then extracts data for the individual columns of each row using the method getValueForColumn(), and compare them pairwise. This data from the rows are supposed to implement Comparable. In the end, I think I got it pretty OK, but I'm still an apprentice when it comes to generics black magic. The subclasses of TableModelComparator want to return different objects from getValueForColumn() for different columns in the row, like Long or String. They are all Comparable, but String is Comparable and Long is Comparable. So I needed to specify the abstract method as returning Comparable, since Comparable is not Comparable. But then, for reasons that I do not fully fathom, I could not specify Comparable o1 = getValueForColumn(row1, columns[i]); Comparable o2 = getValueForColumn(row2, columns[i]); int result = o1.compareTo(o2); because the compiler did not accept the call to compareTo(). I did try to sacrifice a black rooster at midnight and walking backwards in a circle three time, to no avail. Maybe the problem was that it was not full moon or that I have no cat. In the end, I casted o1 and o2 to Comparable and suppressed the warning for that cast. If anyone knows what rituals I need to perform to make this work, or -- even better -- how to fix the code properly, please let me know! A brief discussion of wildcards, ?. The meaning of a wildcard is some particular unknown type, meeting the various constraints from "? extends" or "? super". If there is no explicit constraint listed, it is equivalent to "? extends Object". So the meaning of the code above is something Comparable o1 = getValueForColumn(row1, columns[i]); Comparable o2 = getValueForColumn(row2, columns[i]); where S and T are two fresh, unrelated type variables. Concretely, this means S and T could be, say, String and Integer so that is why a call to o1.compareTo(o2) would not be accepted by the compiler since Strings and Integer aren't comparable to each other. While two instances of "Comparable" are syntactically the same, they don't represent the same type inside the compiler. In some cases, you can get around this issue by using a separate method to capture the type variable and then use it more than once. A technique we've used in the JDK is to have a pubic method with a wildcards calling a private method using a type variable. I've looked around for a few minutes and didn't find a concrete example in the JDK, but they're in there. I haven't looked over the specific code here in much detail; the type Comparable might to be the best you can do, but it isn't very expressive since Object's aren't necessarily comparable. HTH, -Joe
Re: RFR: JDK-8242302 : Refactor jpackage native code
(adding build-dev) Hello Alexey, The SetupJdkExecutable and SetupJdkLibrary macros are designed to find the correct source dirs automatically as long as they follow the standard naming. In this case you are adding some extra with the "common" dir as well as reusing some src dirs between multiple calls to the macros. There are constructs prepared for handling these situations too. We introduced these specialized macros to avoid repeated setting up of almost identical source dirs like in your patch, and to help enforce a standard in directory naming. Upon closer inspection, I see now that what's said above only applies to SetupJdkLibrary while SetupJdkExecutable is lacking most of these features. Adding them should be pretty straight forward though and should be fixed. Not asking you to do it though. So for those cases, I recommend using the FindSrcDirsForComponent macro to find the source dirs needed for now. It's defined in make/common/JdkNativeCompilation.gmk. You can also extract the exact resolved SRC dirs from a call to either macro using for example BUILD_JPACKAGE_APPLAUNCHEREXE_SRC (after the macro has been evaled). This would be recommended for BUILD_JPACKAGE_APPLAUNCHERWEXE. For the ones where the NAME field matches the source sub dir (which should be almost all unless there is a good reason), you don't need to specify SRC at all. If you need to add the "common" subdir, then you can use EXTRA_SRC and the special designation "jdk.incubator.jpackage:common" which will get parsed into all existing common directories for your current build target. In the case of jpackageapplauncherw, you can similarly use "jdk.incubator.jpackage:applauncher" in the SRC field. For SetupJdkLibrary, there is no need to explicitly add source dirs to -I paths. All source dirs are by default added unless HEADERS_FROM_SRC is set to false. Again this should be fixed for SetupJdkExecutable. For macros that we intend to call with parameters, our style convention is to declare them with camel case and always on a new line. This is to make them stand out clearly from variables which we assign with :=, and make them look a little bit more like function declarations. See make/common/Utils.gmk for examples of how this looks. So something like: JpackageWithStaticCrt = \ $(filter-out -MD, $1) -MT /Erik On 2020-04-15 13:13, Alexey Semenyuk wrote: Please review fix [2] for jpackage bug [1]. Refactor jpackage native code. - Improve code reuse between different platforms. - Replace custom string classes with the standard std::basic_string. - Merge functionality of libapplauncher.dll(.so) library with jpackageapplauncher(.exe) binary. There is no point to keep two different executables. - Link jpackageapplauncher.exe with MSVC Run-Time library statically to avoid copying of MSVC Run-Time dlls to app's bin folder. - Remove unused code. - Alexey [1] https://bugs.openjdk.java.net/browse/JDK-8242302 [2] http://cr.openjdk.java.net/~asemenyuk/8242302/webrev.00
Re: RFR: JDK-8243156 Fix deprecation and unchecked warnings in microbenchmark
I've looked at the benchmarks that move from explicit constructors to autoboxing and don't think we'll see any difference in behavior: all usage during setup, and benchmarks don't seem to accidentally depend on identity. LGTM. /Claes On 2020-04-20 15:01, Erik Joelsson wrote: This looks good to me, but I would like to hear from someone who knows the benchmarks to confirm that nothing relevant to the tests was lost when introducing autoboxing instead of explicit constructors in any of the cases. /Erik On 2020-04-20 04:51, Magnus Ihse Bursie wrote: This patch addresses the warnings deprecation and unchecked, which are impossible to fully suppress. With this patch, the test-image can be built fully without warnings. The patch updates non-critical code to use modern solutions for deprecated methods, where possible. Tested methods have been annotated with SuppressWarnings instead. I'm leaving it for a future update by the perf team to reconsider if it's worth testing deprecated methods. Bug: https://bugs.openjdk.java.net/browse/JDK-8243156 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243156-fix-warnings-in-microbenchmark/webrev.01 /Magnus
Re: RFR: JDK-8243156 Fix deprecation and unchecked warnings in microbenchmark
This looks good to me, but I would like to hear from someone who knows the benchmarks to confirm that nothing relevant to the tests was lost when introducing autoboxing instead of explicit constructors in any of the cases. /Erik On 2020-04-20 04:51, Magnus Ihse Bursie wrote: This patch addresses the warnings deprecation and unchecked, which are impossible to fully suppress. With this patch, the test-image can be built fully without warnings. The patch updates non-critical code to use modern solutions for deprecated methods, where possible. Tested methods have been annotated with SuppressWarnings instead. I'm leaving it for a future update by the perf team to reconsider if it's worth testing deprecated methods. Bug: https://bugs.openjdk.java.net/browse/JDK-8243156 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243156-fix-warnings-in-microbenchmark/webrev.01 /Magnus
Re: RFR: JDK-8243154 Fix deprecation warnings in failure handler
Looks good. /Erik On 2020-04-20 02:20, Magnus Ihse Bursie wrote: The failure handler has two places where deprecated methods are used. This results in warnings that are not possible to suppress. This patch fixes the code in those places. This patch is part of the ongoing effort to make the entire build free of warnings. Bug: https://bugs.openjdk.java.net/browse/JDK-8243154 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243154-fix-failurehandler-deprecations/webrev.01 /Magnus
RFR: JDK-8243156 Fix deprecation and unchecked warnings in microbenchmark
This patch addresses the warnings deprecation and unchecked, which are impossible to fully suppress. With this patch, the test-image can be built fully without warnings. The patch updates non-critical code to use modern solutions for deprecated methods, where possible. Tested methods have been annotated with SuppressWarnings instead. I'm leaving it for a future update by the perf team to reconsider if it's worth testing deprecated methods. Bug: https://bugs.openjdk.java.net/browse/JDK-8243156 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243156-fix-warnings-in-microbenchmark/webrev.01 /Magnus
Re: RFR: JDK-8243154 Fix deprecation warnings in failure handler
Hi Magnus, LGTM! Thanks, David On 20/04/2020 7:20 pm, Magnus Ihse Bursie wrote: The failure handler has two places where deprecated methods are used. This results in warnings that are not possible to suppress. This patch fixes the code in those places. This patch is part of the ongoing effort to make the entire build free of warnings. Bug: https://bugs.openjdk.java.net/browse/JDK-8243154 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243154-fix-failurehandler-deprecations/webrev.01 /Magnus
RFR: JDK-8243154 Fix deprecation warnings in failure handler
The failure handler has two places where deprecated methods are used. This results in warnings that are not possible to suppress. This patch fixes the code in those places. This patch is part of the ongoing effort to make the entire build free of warnings. Bug: https://bugs.openjdk.java.net/browse/JDK-8243154 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243154-fix-failurehandler-deprecations/webrev.01 /Magnus
