Re: RFR(S): 8252407: Build failure with gcc-8+ and asan
Hi Kim, Sorry for the delay. This patch removes a redundant string copy in NetworkInterface.c to avoid string-truncation warning. Other warnings we talked before, which are unable to completely fix in different version of gcc, I have to use pragma to suppress them as a workaround. This patch now could compile with gcc-7, gcc-8, gcc-9, gcc-10 both with or without asan. [TESTS] Jtreg: hotspot::hotspot_all_no_apps, jdk::jdk_core and langtools::tier1. No new failure found. http://cr.openjdk.java.net/~qfeng/ericliu/jdk/stringop_trunc/webrev.01/ https://bugs.openjdk.java.net/browse/JDK-8252407 Thanks, Eric
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Tue, 22 Sep 2020 08:43:04 GMT, Erik Gahlin wrote: >> Marked as reviewed by kvn (Reviewer). > > Have you run the JFR tests in test/jdk/jdk/jfr? @marschall Please do not force-push anything as it breaks the commit history in the PR and renders previous reviews/comments obsolete. There is no way for the reviewers to see what changed between the commit they reviewed and the commit now in the PR. To update to latest changes you should have just merged your branch with your (up-to-date) local master, committed that merge in your local branch and then pushed that commit to your Personal Fork. The skara tooling will flatten the series of commits in a PR into one single well-formed commit that is pushed to the master branch of the repo. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v4]
> Hello, newbie here > > I picked JDK-8138732 to work on because it has a "starter" label and I > believe I understand what to do. > > - I tried to update the copyright year to 2020 in every file. > - I decided to change `@since` from 9 to 16 since it is a new annotation name > in a new package. > - I tried to keep code changes to a minimum, eg not change to imports if > fully qualified class names are used instead of > imports. In some cases I did minor reordering of imports to keep them > sorted alphabetically. > - All tier1 tests pass. > - One jpackage/jlink tier2 test fails but I don't believe it is related. > - Some tier3 Swing tests fail but I don't think they are related. Philippe Marschall has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate - Changes: https://git.openjdk.java.net/jdk/pull/45/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=45&range=03 Stats: 753 lines in 65 files changed: 153 ins; 149 del; 451 mod Patch: https://git.openjdk.java.net/jdk/pull/45.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/45/head:pull/45 PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8246774: Record Classes (final) implementation [v3]
> Co-authored-by: Vicente Romero > Co-authored-by: Harold Seigel > Co-authored-by: Jonathan Gibbons > Co-authored-by: Brian Goetz > Co-authored-by: Maurizio Cimadamore > Co-authored-by: Joe Darcy > Co-authored-by: Chris Hegarty > Co-authored-by: Jan Lahoda Vicente Romero has updated the pull request incrementally with three additional commits since the last revision: - Merge pull request #1 from ChrisHegarty/record-serial-tests Remove preview args from JDK tests - Remove preview args from ObjectMethodsTest - Remove preview args from record serialization tests - Changes: - all: https://git.openjdk.java.net/jdk/pull/290/files - new: https://git.openjdk.java.net/jdk/pull/290/files/543e5054..26b80775 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=290&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=290&range=01-02 Stats: 95 lines in 21 files changed: 0 ins; 35 del; 60 mod Patch: https://git.openjdk.java.net/jdk/pull/290.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/290/head:pull/290 PR: https://git.openjdk.java.net/jdk/pull/290
Re: RFR: 8246774: Record Classes (final) implementation
good catch Chris, thanks for the patch, Vicente On 9/22/20 5:51 AM, Chris Hegarty wrote: On Mon, 21 Sep 2020 23:21:18 GMT, Vicente Romero wrote: Hi Vicente, Please file a separate subtask for the javax.lang.model changes. This helps with the JSR 269 MR paperwork. Thanks, -Joe note: I have removed from the original patch the code related to javax.lang.model, I will publish them in a separate PR @vicente-romero-oracle I noticed that we can also remove the preview args from the record serialization tests and ObjectMethodsTest. I opened a PR against the branch in your fork. You should be able to just merge in the changes. See https://github.com/vicente-romero-oracle/jdk/pull/1 - PR: https://git.openjdk.java.net/jdk/pull/290
Re: RFR: 8218685: jlink --list-plugins needs to be readable and tidy [v2]
> These changes update the jlink plugin command line documentation to tidy them > up into a more canonical form. > > The output generated by jlink from this change appears as follows: > >> build/macosx-x64/images/jdk/bin/jlink --list-plugins > > List of available plugins: > --strip-debug Strip debug information from the output image > --strip-java-debug-attributes > Strip Java debug attributes from classes in the > output image > --exclude-resources > Specify resources to exclude. > e.g.: **.jcov,glob:**/META-INF/** > --exclude-files > Specify files to exclude. > e.g.: **.java,glob:/java.base/lib/client/** > --exclude-jmod-section > Specify a JMOD section to exclude. > Where is "man" or "headers". > --dedup-legal-notices [error-if-not-same-content] > De-duplicate all legal notices. > If error-if-not-same-content is specified then > it will be an error if two files of the same > filename are different. > --system-modules retainModuleTarget > Fast loading of module descriptors (always > enabled) > --strip-native-commands Exclude native commands (such as java/java.exe) > from the image > --order-resources > Order resources. > e.g.: > **/module-info.class,@classlist,/java.base/java/lang/** > --compress <0|1|2>[:filter=] > Compress all resources in the output image. > Level 0: No compression > Level 1: Constant string sharing > Level 2: ZIP. > An optional filter can be > specified to list the pattern of files to be > included. > --vm > Select the HotSpot VM in the output image. > Default is all > --include-locales [,]* > BCP 47 language tags separated by a comma, > allowing > locale matching defined in RFC 4647. e.g.: > en,ja,*-IN > --generate-jli-classes @filename > Specify a file listing the java.lang.invoke > classes > to pre-generate. By default, this plugin may use a > builtin list of classes to pre-generate. If this > plugin > runs on a different runtime version than the > image being > created then code generation will be disabled by > default to guarantee correctness > add ignore-version=true to override this. > --release-info |add:=:=:...|del: > option is to load release properties from > the supplied file. > add: is to add properties to the release file. > Any number of = pairs can be passed. > del: is to delete the list of keys in release > file. > --add-optionsPrepend the specified string, which may > include whitespace, before any other options when > invoking the virtual machine in the resulting > image. > --vendor-bug-url > Override the vendor bug URL baked into the build. > The value of the system property > "java.vendor.url.bug" will be . > --vendor-vm-bug-url > Override the vendor VM bug URL baked into the > build. > The URL displayed in VM error logs will be > . > --vendor-version > Override the vendor version string baked into the > build, > if any. The value of the system property > "java.vendor.version" will be . > > For options requiring a , the value will be a comma separated > list of elements each using one the following forms: > > glob: > regex: > @ where filename is the name of a file containing patterns to be > used, one pattern per line Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Fixing up ordering - Changes: - all: https://git.openjdk.java.net/jdk/pull/305/files - new: https://git.openjdk.java.net/jdk/pull/305/files/4add876d..a94a16bd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=305&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=305&range=00-01 Stats: 15 lines in 1 file changed: 3 ins; 10 del; 2 mod Patch: https://git.openjdk.java.net/jdk
Re: RFR: 8218685: jlink --list-plugins needs to be readable and tidy
On Wed, 23 Sep 2020 00:37:18 GMT, Mandy Chung wrote: >> These changes update the jlink plugin command line documentation to tidy >> them up into a more canonical form. >> >> The output generated by jlink from this change appears as follows: >> >>> build/macosx-x64/images/jdk/bin/jlink --list-plugins >> >> List of available plugins: >> --strip-debug Strip debug information from the output image >> --strip-java-debug-attributes >> Strip Java debug attributes from classes in the >> output image >> --exclude-resources >> Specify resources to exclude. >> e.g.: **.jcov,glob:**/META-INF/** >> --exclude-files >> Specify files to exclude. >> e.g.: **.java,glob:/java.base/lib/client/** >> --exclude-jmod-section >> Specify a JMOD section to exclude. >> Where is "man" or "headers". >> --dedup-legal-notices [error-if-not-same-content] >> De-duplicate all legal notices. >> If error-if-not-same-content is specified then >> it will be an error if two files of the same >> filename are different. >> --system-modules retainModuleTarget >> Fast loading of module descriptors (always >> enabled) >> --strip-native-commands Exclude native commands (such as java/java.exe) >> from the image >> --order-resources >> Order resources. >> e.g.: >> **/module-info.class,@classlist,/java.base/java/lang/** >> --compress <0|1|2>[:filter=] >> Compress all resources in the output image. >> Level 0: No compression >> Level 1: Constant string sharing >> Level 2: ZIP. >> An optional filter can be >> specified to list the pattern of files to be >> included. >> --vm >> Select the HotSpot VM in the output image. >> Default is all >> --include-locales [,]* >> BCP 47 language tags separated by a comma, >> allowing >> locale matching defined in RFC 4647. e.g.: >> en,ja,*-IN >> --generate-jli-classes @filename >> Specify a file listing the java.lang.invoke >> classes >> to pre-generate. By default, this plugin may use >> a >> builtin list of classes to pre-generate. If this >> plugin >> runs on a different runtime version than the >> image being >> created then code generation will be disabled by >> default to guarantee correctness >> add ignore-version=true to override this. >> --release-info |add:=:=:...|del:> list> >> option is to load release properties from >> the supplied file. >> add: is to add properties to the release file. >> Any number of = pairs can be passed. >> del: is to delete the list of keys in release >> file. >> --add-optionsPrepend the specified string, which may >> include whitespace, before any other options when >> invoking the virtual machine in the resulting >> image. >> --vendor-bug-url >> Override the vendor bug URL baked into the build. >> The value of the system property >> "java.vendor.url.bug" will be . >> --vendor-vm-bug-url >> Override the vendor VM bug URL baked into the >> build. >> The URL displayed in VM error logs will be >> . >> --vendor-version >> Override the vendor version string baked into >> the build, >> if any. The value of the system property >> "java.vendor.version" will be . >> >> For options requiring a , the value will be a comma separated >> list of elements each using one the following forms: >> >> glob: >> regex: >> @ where filename is the name of a file containing patterns to be >> used, one pattern per line > > The output looks much better. Have you considered to sort them in > alphabetical order of the plugin name? Yes! I had intended to but it looks like I got hung up making sure non-documented plugins came last. Will push a change for this. - PR: https://git.openjdk.java.net/jdk/pull/305
Re: Jpackage file assocations OS X
> On Sep 22, 2020, at 7:35 PM, alexander.matv...@oracle.com wrote: > > Hi Michael, > > For file association you will need to create property file and pass it to > jpackage via --file-associations. Thanks for the detailed reply. I will look closer at it later. I had found the test class. I was looking in the incubator directory and not tool previously. Between your reply and that should be able to understand how it should work OS X.
Re: RFR: 8218685: jlink --list-plugins needs to be readable and tidy
On Tue, 22 Sep 2020 17:52:00 GMT, Ian Graves wrote: > These changes update the jlink plugin command line documentation to tidy them > up into a more canonical form. > > The output generated by jlink from this change appears as follows: > >> build/macosx-x64/images/jdk/bin/jlink --list-plugins > > List of available plugins: > --strip-debug Strip debug information from the output image > --strip-java-debug-attributes > Strip Java debug attributes from classes in the > output image > --exclude-resources > Specify resources to exclude. > e.g.: **.jcov,glob:**/META-INF/** > --exclude-files > Specify files to exclude. > e.g.: **.java,glob:/java.base/lib/client/** > --exclude-jmod-section > Specify a JMOD section to exclude. > Where is "man" or "headers". > --dedup-legal-notices [error-if-not-same-content] > De-duplicate all legal notices. > If error-if-not-same-content is specified then > it will be an error if two files of the same > filename are different. > --system-modules retainModuleTarget > Fast loading of module descriptors (always > enabled) > --strip-native-commands Exclude native commands (such as java/java.exe) > from the image > --order-resources > Order resources. > e.g.: > **/module-info.class,@classlist,/java.base/java/lang/** > --compress <0|1|2>[:filter=] > Compress all resources in the output image. > Level 0: No compression > Level 1: Constant string sharing > Level 2: ZIP. > An optional filter can be > specified to list the pattern of files to be > included. > --vm > Select the HotSpot VM in the output image. > Default is all > --include-locales [,]* > BCP 47 language tags separated by a comma, > allowing > locale matching defined in RFC 4647. e.g.: > en,ja,*-IN > --generate-jli-classes @filename > Specify a file listing the java.lang.invoke > classes > to pre-generate. By default, this plugin may use a > builtin list of classes to pre-generate. If this > plugin > runs on a different runtime version than the > image being > created then code generation will be disabled by > default to guarantee correctness > add ignore-version=true to override this. > --release-info |add:=:=:...|del: > option is to load release properties from > the supplied file. > add: is to add properties to the release file. > Any number of = pairs can be passed. > del: is to delete the list of keys in release > file. > --add-optionsPrepend the specified string, which may > include whitespace, before any other options when > invoking the virtual machine in the resulting > image. > --vendor-bug-url > Override the vendor bug URL baked into the build. > The value of the system property > "java.vendor.url.bug" will be . > --vendor-vm-bug-url > Override the vendor VM bug URL baked into the > build. > The URL displayed in VM error logs will be > . > --vendor-version > Override the vendor version string baked into the > build, > if any. The value of the system property > "java.vendor.version" will be . > > For options requiring a , the value will be a comma separated > list of elements each using one the following forms: > > glob: > regex: > @ where filename is the name of a file containing patterns to be > used, one pattern per line The output looks much better. Have you considered to sort them in alphabetical order of the plugin name? - PR: https://git.openjdk.java.net/jdk/pull/305
Re: Jpackage file assocations OS X
Hi Michael, For file association you will need to create property file and pass it to jpackage via --file-associations. Example property file: mime-type=text/plain extension=txt description=Text file See following documentation: https://docs.oracle.com/en/java/javase/15/jpackage/support-application-features.html#GUID-8668A806-8A80-435F-970F-7B2BF65863E4 To retrieve associated file(s) when application is invoked by clicking associated file in Finder use OpenFilesHandler from AWT. See https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/java/awt/Desktop.html#setOpenFileHandler(java.awt.desktop.OpenFilesHandler) Also, for example on how to use above APIs you can refer to test app we using for jpackage: https://github.com/openjdk/jdk/blob/master/test/jdk/tools/jpackage/apps/image/Hello.java We also support CFBundleDocumentTypes from https://developer.apple.com/documentation/bundleresources/information_property_list/cfbundledocumenttypes?language=objc just add then to property file mac.key=value with key one of CFBundleDocumentTypes key and correct value from above doc. Example: mime-type=text/plain extension=txt description=Text file mac.CFBundleTypeRole=Viewer mac.LSHandlerRank=Default mac.NSDocumentClass=SomeClass mac.LSTypeIsPackage=true mac.LSSupportsOpeningDocumentsInPlace=false mac.UISupportsDocumentBrowser=false mac.NSExportableTypes=public.png, public.jpg mac.UTTypeConformsTo=public.image, public.data CFBundleDocumentTypes was added with https://bugs.openjdk.java.net/browse/JDK-8233215 Thanks, Alexander On 9/22/20 12:00 PM, Michael Hall wrote: On Sep 22, 2020, at 1:47 PM, Andy Herrick wrote: Alexander: Can you explain the mechanism by which a macosx application is expected to retrieve the path to the selected file when the app is invoked via a file association ? On mac, the path to the associated file is not passed as an argument (as it is on Linux and Windows). /Andy On 9/20/2020 8:40 PM, Michael Hall wrote: Are there any examples or further information on how the file association property file should work? It is not entirely clear to me from the command help. Is this currently not intended to be supported on OS X at this time? As I remember you have the extensions and what role they have for the application and things like that. These being entries in the Info.plist. It wasn’t clear to me how that would be applied in a properties file.
Re: RFR: 8253149: Building an installer from invalid app image fails on Window…
On Sun, 20 Sep 2020 21:23:17 GMT, Andy Herrick wrote: > 8253149: Building an installer from invalid app image fails on Windows and > Linux > When jpackage builds a package from an app-image that was not generated by > jpackage, the tool should give user a > warning message, and then complete the package anyway. OK - I spelled foreign with a different wrong spelling (and missed a place in comments of test) will fix them all in the morning. - PR: https://git.openjdk.java.net/jdk/pull/271
Integrated: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
On Wed, 16 Sep 2020 17:29:52 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to the issue wrt missing hashCode() javadoc, which was > recently discussed in core-libs ml. This pull request has now been integrated. Changeset: bddb8225 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/bddb8225 Stats: 6 lines in 1 file changed: 1 ins; 5 del; 0 mod 8253240: No javadoc for DecimalFormatSymbols.hashCode() Reviewed-by: rriggs, lancea - PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8253149: Building an installer from invalid app image fails on Window… [v3]
On Tue, 22 Sep 2020 11:53:14 GMT, Andy Herrick wrote: >> 8253149: Building an installer from invalid app image fails on Windows and >> Linux >> When jpackage builds a package from an app-image that was not generated by >> jpackage, the tool should give user a >> warning message, and then complete the package anyway. > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > 8253149: Building an installer from invalid app image fails (revision 2) test/jdk/tools/jpackage/share/AppImagePackageTest.java line 87: > 85: > 86: // on mac, with --app-image and without > --mac-package-identifier, > 87: // we will try to infer it from the image, so forign image > needs it. forign -> foreign. Same at line 92. - PR: https://git.openjdk.java.net/jdk/pull/271
Re: RFR: 8253149: Building an installer from invalid app image fails on Window… [v3]
On Tue, 22 Sep 2020 11:53:14 GMT, Andy Herrick wrote: >> 8253149: Building an installer from invalid app image fails on Windows and >> Linux >> When jpackage builds a package from an app-image that was not generated by >> jpackage, the tool should give user a >> warning message, and then complete the package anyway. > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > 8253149: Building an installer from invalid app image fails (revision 2) src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/resources/LinuxResources.properties line 64: > 62: message.rpm-ldd-not-available.advice=Install "glibc-common" RPM package > to get ldd. > 63: > 64: warning.foerign-app-image=Warning: app-image dir not generated by > jpackage. I think it is misspelled again. - PR: https://git.openjdk.java.net/jdk/pull/271
Re: RFR 8253451: Performance regression in java.util.Scanner after 8236201
Hi Martin,
Overall it seems reasonable to replace the \x{HH} construct with a simpler escape
sequence. I'm a bit surprised to see the performance impact, but you noticed it, so
I'll buy that it's significant.
// These must be literalized to avoid collision with regex
// metacharacters such as dot or parenthesis
-groupSeparator = "\\x{" + Integer.toHexString(dfs.getGroupingSeparator()) +
"}";
-decimalSeparator = "\\x{" + Integer.toHexString(dfs.getDecimalSeparator()) +
"}";
+char newSep;
+groupSeparator = ((newSep = dfs.getGroupingSeparator()) == ',' ||
+newSep == '.' || newSep == ' ' ? "\\" + newSep :
+"\\x{" + Integer.toHexString(newSep) + "}" );
+decimalSeparator = ((newSep = dfs.getDecimalSeparator()) == '.' ||
+newSep == ',' ? "\\" + newSep : "\\x{" +
+Integer.toHexString(newSep) + "}" );
Fundamentally it's simple, but there's rather a lot going on here:
- reuse of a local variable
- assignment within a logical expression
- odd line breaking
- ternary operator
These factors make the code rather difficult to read, unnecessarily so in my
opinion.
(The collections and concurrency libraries sometimes put assignments within other
expressions, but usually to avoid unnecessary initialization of a local variable. I
don't think that applies here.)
More to the semantics of the operations, the two cases seem oddly different: the
group separator tests for ',' and '.' and ' ', but the decimal separator tests for
'.' and ','. Why are they testing for different things, and in the opposite order?
I looked through the 810 locales available in Oracle JDK 15, and here's what I
found:
Decimal Separators:
U+002c 359
U+002e 400
U+066b 51
Grouping Separators:
U+002c 378
U+002e 167
U+00a0 158
U+066c 51
U+2019 12
U+202f 44
I don't see a space used as a gropuing separator, but note that U+00a0 is no-break
space. Does that occur often enough to special-case?
(Note that these numbers don't take into account how often each locale is used, nor
are the 810 locales in the JDK the universe of all possible locales. However, it
seems likely that any other locales would not appear frequently enough to bother
with a special case.)
I also see that the group separator code tests for ',' first whereas the decimal
separator code tests for '.' first. I suppose this might be a femto-optimization
that slightly favors English-speaking locales, but it seems gratuitous to me. (If
you can come up with a benchmark that illustrates the difference, be my guest!) :-)
I'm wondering if it might be nicer to extract this computation into a little utility
method nearby:
// Escapes a separator character to prevent it from being
// interpreted as a regex metacharacter. > static String
escapeSeparator(char ch) {
if (ch == ',' || ch == '.') {
return "\\" + ch;
} else {
return "\\x{" + Integer.toHexString(ch) + "}";
}
}
(Note that I rewrote the comment that already existed above the lines you changed,
as it seemed misleading.)
The calling code would then be changed to:
groupSeparator = escapeSeparator(dfs.getGroupingSeparator());
decimalSeparator = escapeSeparator(dfs.getDecimalSeparator());
I haven't benchmarked this. However, this would be called from within useLocale(),
which is a low-frequency operation compared to the cases you benchmarked.
Also, given that U+00a0 occurs relatively frequently among locales, it might be
worthwhile also to special-case for it, but I don't have a strong opinion on that.
This case would likely never occur for the decimal separator, but it doesn't bother
me. I don't think it adds much value to have different special cases for the
different separators.
**
Finally, can this be converted into a github PR? It'll have to be made into a PR
sooner or later in order to be integrated into the JDK mainline. (Though I guess the
update releases are still using the current email+webrev process for now, sorry.)
s'marks
On 9/21/20 9:48 PM, Martin Balao wrote:
Hi,
I'd like to propose a fix for JDK-8253451 [1] performance regression.
Webrev.00:
* http://cr.openjdk.java.net/~mbalao/webrevs/8253451/8253451.webrev.00
As explained in [1], the idea for the fix is to optimize the regexp
string for the most common group and decimal separator characters across
locales. Please note that there are a few locales where the proposed
optimization does not apply and the slower -but safe- regexp introduced
by 8236201 is used.
Testing:
* No regressions observed in jdk/java/util/Scanner (5/5 passed)
* Verified performance improvement back to what it was before 8236201
(on my Americas locale).
Thanks,
Martin.-
--
[1] - https://bugs.openjdk.java.net/browse/JDK-8253451
Re: Math.exp() yields different results on 32 bit systems
Hello,
Per the spec
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Math.html#exp(double)
the result of exp has to be within one ulp of the exact result and the
value of e^(10_000) is (much) larger than Double.MAX_VALUE so positive
infinity is the correct answer. Therefore, it looks like the intrinsic
on 32-bit systems is buggy.
HTH,
-Joe
On 9/22/2020 3:30 AM, Andreas Ahlenstorf wrote:
Hi!
According to a bug report at AdoptOpenJDK [1] Math.exp(10_000.0) yields
different results on 32 bit systems than on 64 bit systems.
public class Test {
public static void main(String[] args) {
System.out.println(Math.exp(10_000.0));
}
}
On 64 bit systems the code above prints Infinity. On 32 bit systems the result
is 0.0. I think that's wrong, but I'm far from an expert in floating point
arithmetics.
Affected versions:
* OpenJDK 9.0.1
* OpenJDK 11.0.8
* OpenJDK 15
Not affected:
* OpenJDK 8u265
We tested both with AdoptOpenJDK and Azul Zulu on Windows and Linux on x86 and
x64. If we disable intrinsics (-XX:+UnlockDiagnosticVMOptions
-XX:-UseLibmIntrinsic), Infinity is printed on x86 systems, too.
Is this expected behavior?
Best,
Andreas
[1] https://github.com/AdoptOpenJDK/openjdk-support/issues/182
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode() [v2]
On Wed, 16 Sep 2020 18:02:42 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the fix to the issue wrt missing hashCode() javadoc, which was >> recently discussed in core-libs ml. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Addressing Roger's comments. Looks fine. Thanks - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v4]
On Tue, 22 Sep 2020 07:28:42 GMT, Alan Bateman wrote: > Given that the offset handling is buggy then I think the test should at least > cover the cases where the offset is 0 or > out of bounds. No problem separating out further test coverage for the other > setDictionary methods into a separate > issue. I updated the test to include an offset test for 0 and to add a test for out of range offsets. I have updated the PR. - PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v5]
> Hi all, > > Please review the fix for JDK-8252739 which addresses an issue introduced by > https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored > the offset specified by > Deflater.setDictionary. Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly > as well as the java/util/zip and > java/util/jar JCK tests. Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: Add additional offset test cases in DeflaterDictionaryTests.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/269/files - new: https://git.openjdk.java.net/jdk/pull/269/files/f34a6702..a37d9d28 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=03-04 Stats: 59 lines in 1 file changed: 55 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/269.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/269/head:pull/269 PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
On Tue, 22 Sep 2020 15:36:24 GMT, Daniel Fuchs wrote:
>> No, It is not used.
>> However, I'd like to leave it as is (it is mentioned in the documentation as
>> unsupported value).
>> Otherwise, TlsChannelBindingType enum will have one element only and should
>> be simplified/removed in all places. In
>> this case, it would be double work to add TlsChannelBindingType enum back in
>> the future if "tls-unique" required. If
>> required I can remove TLS_UNIQUE item, but not remove TlsChannelBindingType
>> enum
>
> I was suggesting to keep TlsChannelBindingType but remove TLS_UNIQUE;
> However, I'm OK to keep things as is: this is an
> internal API. I wonder if it would deserve a comment:
> /**
> * Channel binding on the basis of TLS Finished message
> */
> // TLS_UNIQUE is defined by RFC 5929 but is not supported by the
> current LDAP stack.
> TLS_UNIQUE("tls-unique"),
Thank you. Added suggested comment.
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
> Hi, > > Plaese review JDK-8245527 fix which implements LDAP Channel Binding support > for Java GSS/Kerberos. > Initial review is available at core-devs: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html > This version removes "tls-unique" CB type from the list of possible channel > binding types. The only supported type is > "tls-server-end-point" > CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 > > Thank you > Alexey Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision: 8245527: version.01 - Changes: - all: https://git.openjdk.java.net/jdk/pull/278/files - new: https://git.openjdk.java.net/jdk/pull/278/files/3f4ae08c..8b135f48 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=278&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=278&range=00-01 Stats: 15 lines in 4 files changed: 7 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/278/head:pull/278 PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
On Tue, 22 Sep 2020 15:11:57 GMT, Weijun Wang wrote:
>> Alexey Bakhtin has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8245527: version.01
>
> src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Client.java
> line 156:
>
>> 154: if (props != null) {
>> 155: // TLS Channel Binding
>> 156: byte[] tlsCB =
>> (byte[])props.get("jdk.internal.sasl.tlschannelbinding");
>
> You can say the name is defined in another class in another module. If we
> really want to rename it one day we will know
> where it's from.
Thank you. Comment is added
> src/java.security.jgss/share/classes/sun/security/jgss/krb5/InitialToken.java
> line 389:
>
>> 387: int acceptorAddressType = getAddrType(acceptorAddress,
>> 388: (channelBinding instanceof TlsChannelBindingImpl)?
>> 389:
>> CHANNEL_BINDING_AF_UNSPEC:CHANNEL_BINDING_AF_NULL_ADDR);
>
> Normally we put a white space around "?" and ":".
Thank you. Fixed.
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
> line 82:
>
>> 80: /**
>> 81: * Parse value of "com.sun.jndi.ldap.tls.cbtype" property
>> 82: * @param cbType
>
> Please add a `@return` here, esp, about null.
Added @return with comments
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
> line 137:
>
>> 135: public TlsChannelBindingType getType() {
>> 136: return cbType;
>> 137: }
>
> Add a new line here.
Fixed
-
PR: https://git.openjdk.java.net/jdk/pull/278
Integrated: 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class
On Tue, 22 Sep 2020 19:48:33 GMT, Ioi Lam wrote: > Please review this BACKOUT for tier-1 failures. I have tested all CDS tests > locally on Linux-x64, including the failed > DeterministicDump.java. > I would like to apply for the "trivial" rule since this is a simple backout > for tier-1 failures. > > I executed the command: > > git revert c1df13b855984dc2197c0561f7a50c7a5a731dd9 > > and them manually fixed the conflict with > 7b860120e130c13de8ba543a8706c9ecdeab468b. This pull request has now been integrated. Changeset: 65af8373 Author:Ioi Lam URL: https://git.openjdk.java.net/jdk/commit/65af8373 Stats: 156 lines in 22 files changed: 57 ins; 53 del; 46 mod 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class Reviewed-by: eosterlund, dcubed - PR: https://git.openjdk.java.net/jdk/pull/308
Re: RFR: 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class
On Tue, 22 Sep 2020 19:48:33 GMT, Ioi Lam wrote: > Please review this BACKOUT for tier-1 failures. I have tested all CDS tests > locally on Linux-x64, including the failed > DeterministicDump.java. > I would like to apply for the "trivial" rule since this is a simple backout > for tier-1 failures. > > I executed the command: > > git revert c1df13b855984dc2197c0561f7a50c7a5a731dd9 > > and them manually fixed the conflict with > 7b860120e130c13de8ba543a8706c9ecdeab468b. I compared the full webrev patch for this fix with the full webrev patch for JDK-8253208. This looks like a proper backout/revert. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/308
Re: RFR: 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class
On Tue, 22 Sep 2020 19:48:33 GMT, Ioi Lam wrote: > Please review this BACKOUT for tier-1 failures. I have tested all CDS tests > locally on Linux-x64, including the failed > DeterministicDump.java. > I would like to apply for the "trivial" rule since this is a simple backout > for tier-1 failures. > > I executed the command: > > git revert c1df13b855984dc2197c0561f7a50c7a5a731dd9 > > and them manually fixed the conflict with > 7b860120e130c13de8ba543a8706c9ecdeab468b. Marked as reviewed by eosterlund (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/308
RFR: 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class
Please review this BACKOUT for tier-1 failures. I have tested all CDS tests locally on Linux-x64, including the failed DeterministicDump.java. I would like to apply for the "trivial" rule since this is a simple backout for tier-1 failures. I executed the command: git revert c1df13b855984dc2197c0561f7a50c7a5a731dd9 and them manually fixed the conflict with 7b860120e130c13de8ba543a8706c9ecdeab468b. - Commit messages: - [BACKOUT] JDK-8253208 Move CDS related code to a separate class Changes: https://git.openjdk.java.net/jdk/pull/308/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=308&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253496 Stats: 156 lines in 22 files changed: 53 ins; 57 del; 46 mod Patch: https://git.openjdk.java.net/jdk/pull/308.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/308/head:pull/308 PR: https://git.openjdk.java.net/jdk/pull/308
Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v3]
> # JDK-8252523: Add ASN.1 Formatter to work with test utility HexPrinter > > Debugging functions that utilize ASN.1, DER, and BER encoded streams is > difficult without test utilities to show the contents. > The ASN.1 formatter reads a stream and produces annotated output of the > tags, values, and structures. > When used with the test library jdk.test.lib.hexdump.HexPrinter the > annotations are synchronized > with the hex formatted output. > > Small changes to HexPrinter are included to improve the output readability. > > > Example decoding of a .pem certificate: > SEQUENCE [910] > SEQUENCE [630] > CONTEXT cons 0 [3] > BYTE 2, > BYTE 3, > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > NULL > SEQUENCE [76] > SET [11] > SEQUENCE [9] > OBJECT ID [3] 2.5.4.6 (CountryName) > 'IN' > ... > SET [16] > SEQUENCE [14] > OBJECT ID [3] 2.5.4.3 (CommonName) > Client1 > SEQUENCE [30] > UTCTIME [13] '150526221718Z' > UTCTIME [13] '250523221718Z' > ... > SEQUENCE [290] > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.1 (RSA) > NULL > BIT STRING [271] > CONTEXT cons 3 [123] > SEQUENCE [121] > SEQUENCE [9] > OBJECT ID [3] 2.5.29.19 (BasicConstraints) > OCTET STRING [2] > SEQUENCE [44] > OBJECT ID [9] 2.16.840.1.113730.1.13 > OCTET STRING [31] '..OpenSSL Generated Certificate' > SEQUENCE [29] > OBJECT ID [3] 2.5.29.14 (SubjectKeyID) > OCTET STRING [22] > SEQUENCE [31] > OBJECT ID [3] 2.5.29.35 (AuthorityKeyID) > OCTET STRING [24] > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > NULL > BIT STRING [257] > When used with the HexPrinter test utility, the formatting of the > hexadecimal values is selected with the parameters to HexPrinter. > > : 30 82 03 8e ; SEQUENCE [910] > 0004: 30 82 02 76 ; SEQUENCE [630] > 0008: a0 03 ; CONTEXT cons > 0 [3] > 000a: 02 01 02 ; BYTE 2, > 000d:02 01 03 ; BYTE 3, > 0010: 30 0d ; SEQUENCE [13] > 0012: 06 09 2a 86 48 86 f7 0d 01 01 0b ; OBJECT ID > [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > 001d:05 00; NULL > 001f: 30 ; SEQUENCE [76] > 0020: 4c ; > 0021:31 0b; SET [11] > 0023: 30 09 ; SEQUENCE > [9] > 0025:06 03 55 04 06 ; OBJECT > ID [3] 2.5.4.6 (CountryName) > 002a: 13 02 49 4e ; 'IN' > > ... ... > > 005b: 31 10 ; SET [16] > 005d:30 0e; SEQUENCE > [14] > 005f: 06 ; OBJECT > ID [3] 2.5.4.3 (CommonName) > 0060: 03 55 04 03 ; > 0064: 0c 07 43 6c 69 65 6e 74 31 ; Client1 > 006d:30 1e; SEQUENCE [30] > 006f: 17 ; UTCTIME > [13] '150526221718Z' > 0070: 0d 31 35 30 35 32 36 32 32 31 37 31 38 5a ; > 007e: 17 0d ; UTCTIME > [13] '250523221718Z' > 0080: 32 35 30 35 32 33 32 32 31 37 31 38 5a ; > > ... ... > > 00db: 30 82 01 22; SEQUENCE [290] > 00df: 30 ; SEQUENCE > [13] > 00e0: 0d ; > 00e1:06 09 2a 86 48 86 f7 0d 01 01 01 ; OBJECT ID > [9] 1.2.840.113549.1.1.1 (RSA) > 00ec: 05 00 ; NULL > 00ee: 03 82 ; BIT STRING > [271] > 00f0: 01 0f 00 30 82 01 0a 02 82 01 01 00 d8 70 03 54 ; > > ... > > 01f0: 0a 2d f5 de 59 3e d9 5e 74 93 d2 45 02 03 01 00 ; > 0200: 01 ; > 0201:a3 7b;
Maybe a new container: Sieve
Dear all,
Thank you for your great job about the openjdk.
I have an idea about a new container, Sieve(I give it the name).
The java file is something like this:
package com.heyifei.studyandshare.utils;
/**
* @author: ??(heyifei)
* @email: heyife...@foxmail.com
* @address: zhangjiang, pudong new district, shanghai, china
* @date: 2020/9/21
* @description:
*/
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
/**
* Container:Sieve
*/
public class Sieve implements Serializable {
private static final long serialVersionUID = 8621987070720161207L;
/**
* items in the sieve.
*/
final List
Re: Jpackage file assocations OS X
> On Sep 22, 2020, at 1:47 PM, Andy Herrick wrote: > > Alexander: > > Can you explain the mechanism by which a macosx application is expected to > retrieve the path to the selected file when the app is invoked via a file > association ? > > On mac, the path to the associated file is not passed as an argument (as it > is on Linux and Windows). > > /Andy > > On 9/20/2020 8:40 PM, Michael Hall wrote: >> Are there any examples or further information on how the file association >> property file should work? >> It is not entirely clear to me from the command help. >> >> Is this currently not intended to be supported on OS X at this time? As I remember you have the extensions and what role they have for the application and things like that. These being entries in the Info.plist. It wasn’t clear to me how that would be applied in a properties file.
Re: Jpackage file assocations OS X
Alexander: Can you explain the mechanism by which a macosx application is expected to retrieve the path to the selected file when the app is invoked via a file association ? On mac, the path to the associated file is not passed as an argument (as it is on Linux and Windows). /Andy On 9/20/2020 8:40 PM, Michael Hall wrote: Are there any examples or further information on how the file association property file should work? It is not entirely clear to me from the command help.
Re: RFR: 8242451: ensure semantics of non-capturing lambdas are preserved independent of execution mode [v4]
On Tue, 22 Sep 2020 18:24:02 GMT, Gilles Duboscq wrote: > Thanks @mlchung. Do I need a second review? No need. You can integrate once you run the regression tests (I usually run tier1-tier3). - PR: https://git.openjdk.java.net/jdk/pull/93
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v4]
> This is a continuation of > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html > > Changes since then: > * We've improved the write barrier as suggested by Andrew [1] > * The define-guards around R18 have been changed to `R18_RESERVED`. This will > be enabled for Windows only for now but > will be required for the upcoming macOS+Aarch64 [2] port as well. > * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov > in our PR for now and built the > Windows-specific CPU feature detection on top of it. > > [1] > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html > [2] https://openjdk.java.net/jeps/8251280 Monica Beckwith has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits: - 8248787: G1: Workaround MSVC bug Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248670: Windows: Exception handling support on AArch64 Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248660: AArch64: Make _clear_cache and _nop portable Summary: __builtin___clear_cache, etc. Contributed-by: mbeckwit, luhenry, burban - 8248659: AArch64: Extend CPU Feature detection Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248656: Add Windows AArch64 platform support code Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248498: Add build system support for Windows AArch64 Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248681: AArch64: MSVC doesn't support __PRETTY_FUNCTION__ Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248663: AArch64: Avoid existing macros/keywords of MSVC Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - 8248676: AArch64: Add workaround for LITable constructor Reviewed-by: aph Contributed-by: mbeckwit, luhenry, burban - 8248500: AArch64: Remove the r18 dependency on Windows AArch64 (regenerate tests) Reviewed-by: Contributed-by: mbeckwit, luhenry, burban - ... and 3 more: https://git.openjdk.java.net/jdk/compare/7b860120...50ab8edf - Changes: https://git.openjdk.java.net/jdk/pull/212/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=212&range=03 Stats: 2970 lines in 69 files changed: 2407 ins; 275 del; 288 mod Patch: https://git.openjdk.java.net/jdk/pull/212.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/212/head:pull/212 PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8242451: ensure semantics of non-capturing lambdas are preserved independent of execution mode [v4]
On Tue, 22 Sep 2020 18:17:49 GMT, Mandy Chung wrote: >> Gilles Duboscq has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental >> views will show differences compared to the previous content of the PR. The >> pull request contains six new commits since >> the last revision: >> - Move LambdaEagerInitTest to test/jdk/java/lang/invoke/lambda >> - Include capturing case test, use jdk.test.lib.Assert >> - Remove disableEagerInitialization concerns from BridgeMethod.java >> - Remove extra field test from LambdaTest6 >> - Wrap long lines >> - Add dedicated test in the jdk tests > > Looks good. Thanks for the update. Thanks @mlchung. Do I need a second review? - PR: https://git.openjdk.java.net/jdk/pull/93
Integrated: 8253492: Miss comma after second copyright year in FDBigInteger.java
On Tue, 22 Sep 2020 18:06:40 GMT, Yumin Qi wrote: > In patch for jdk-8253208, a comma missed after copyright year in > src/java.base/share/classes/jdk/internal/math/FDBigInteger.java. Also changed > "@return" to "Returns" in the comment for > CDS.getRandomSeedForDumping to keep consistent with others. This pull request has now been integrated. Changeset: 7b860120 Author:Yumin Qi URL: https://git.openjdk.java.net/jdk/commit/7b860120 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod 8253492: Miss comma after second copyright year in FDBigInteger.java Reviewed-by: ccheung - PR: https://git.openjdk.java.net/jdk/pull/306
Re: RFR: 8242451: ensure semantics of non-capturing lambdas are preserved independent of execution mode [v4]
On Tue, 22 Sep 2020 18:03:37 GMT, Gilles Duboscq wrote: >> [JDK-8232806](https://bugs.openjdk.java.net/browse/JDK-8232806) introduced >> the >> jdk.internal.lambda.disableEagerInitialization system property to be able to >> disable eager initialization of lambda >> classes. This was necessary to prevent side effects of class initializers >> triggered by such initialization in the >> context of the GraalVM native image tool. However, the change as it is >> implemented means that the behaviour of >> non-capturing lambdas depends on the value of `disableEagerInitialization`: >> when it is false (the default) such lambdas >> are actually a singleton while when it is true, a fresh instance is returned >> every time. Programs should definitely >> _not_ rely on reference equality since the Java spec does not guarantee it. >> However, in order to separate concern and >> ease debugging such bad programs, `disableEagerInitialization` shouldn't >> influence the singleton vs. fresh instance >> behaviour of lambdas in either direction. > > Gilles Duboscq has refreshed the contents of this pull request, and previous > commits have been removed. The incremental > views will show differences compared to the previous content of the PR. Looks good. Thanks for the update. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/93
Re: RFR: 8253492: Miss comma after second copyright year in FDBigInteger.java
On Tue, 22 Sep 2020 18:06:40 GMT, Yumin Qi wrote: > In patch for jdk-8253208, a comma missed after copyright year in > src/java.base/share/classes/jdk/internal/math/FDBigInteger.java. Also changed > "@return" to "Returns" in the comment for > CDS.getRandomSeedForDumping to keep consistent with others. Looks good and trivial. - Marked as reviewed by ccheung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/306
RFR: 8253492: Miss comma after second copyright year in FDBigInteger.java
In patch for jdk-8253208, a comma missed after copyright year in src/java.base/share/classes/jdk/internal/math/FDBigInteger.java. Also changed "@return" to "Returns" in the comment for CDS.getRandomSeedForDumping to keep consistent with others. - Commit messages: - 8253492: Miss comma after second copyright year in FDBigInteger.java Changes: https://git.openjdk.java.net/jdk/pull/306/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=306&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253492 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/306.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/306/head:pull/306 PR: https://git.openjdk.java.net/jdk/pull/306
Re: RFR: 8242451: ensure semantics of non-capturing lambdas are preserved independent of execution mode [v4]
> [JDK-8232806](https://bugs.openjdk.java.net/browse/JDK-8232806) introduced the > jdk.internal.lambda.disableEagerInitialization system property to be able to > disable eager initialization of lambda > classes. This was necessary to prevent side effects of class initializers > triggered by such initialization in the > context of the GraalVM native image tool. However, the change as it is > implemented means that the behaviour of > non-capturing lambdas depends on the value of `disableEagerInitialization`: > when it is false (the default) such lambdas > are actually a singleton while when it is true, a fresh instance is returned > every time. Programs should definitely > _not_ rely on reference equality since the Java spec does not guarantee it. > However, in order to separate concern and > ease debugging such bad programs, `disableEagerInitialization` shouldn't > influence the singleton vs. fresh instance > behaviour of lambdas in either direction. Gilles Duboscq has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains six new commits since the last revision: - Move LambdaEagerInitTest to test/jdk/java/lang/invoke/lambda - Include capturing case test, use jdk.test.lib.Assert - Remove disableEagerInitialization concerns from BridgeMethod.java - Remove extra field test from LambdaTest6 - Wrap long lines - Add dedicated test in the jdk tests - Changes: - all: https://git.openjdk.java.net/jdk/pull/93/files - new: https://git.openjdk.java.net/jdk/pull/93/files/625feb94..5525f217 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=93&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=93&range=02-03 Stats: 164 lines in 2 files changed: 91 ins; 73 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/93.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/93/head:pull/93 PR: https://git.openjdk.java.net/jdk/pull/93
RFR: 8218685: jlink --list-plugins needs to be readable and tidy
These changes update the jlink plugin command line documentation to tidy them up into a more canonical form. The output generated by jlink from this change appears as follows: > build/macosx-x64/images/jdk/bin/jlink --list-plugins List of available plugins: --strip-debug Strip debug information from the output image --strip-java-debug-attributes Strip Java debug attributes from classes in the output image --exclude-resources Specify resources to exclude. e.g.: **.jcov,glob:**/META-INF/** --exclude-files Specify files to exclude. e.g.: **.java,glob:/java.base/lib/client/** --exclude-jmod-section Specify a JMOD section to exclude. Where is "man" or "headers". --dedup-legal-notices [error-if-not-same-content] De-duplicate all legal notices. If error-if-not-same-content is specified then it will be an error if two files of the same filename are different. --system-modules retainModuleTarget Fast loading of module descriptors (always enabled) --strip-native-commands Exclude native commands (such as java/java.exe) from the image --order-resources Order resources. e.g.: **/module-info.class,@classlist,/java.base/java/lang/** --compress <0|1|2>[:filter=] Compress all resources in the output image. Level 0: No compression Level 1: Constant string sharing Level 2: ZIP. An optional filter can be specified to list the pattern of files to be included. --vm Select the HotSpot VM in the output image. Default is all --include-locales [,]* BCP 47 language tags separated by a comma, allowing locale matching defined in RFC 4647. e.g.: en,ja,*-IN --generate-jli-classes @filename Specify a file listing the java.lang.invoke classes to pre-generate. By default, this plugin may use a builtin list of classes to pre-generate. If this plugin runs on a different runtime version than the image being created then code generation will be disabled by default to guarantee correctness add ignore-version=true to override this. --release-info |add:=:=:...|del: option is to load release properties from the supplied file. add: is to add properties to the release file. Any number of = pairs can be passed. del: is to delete the list of keys in release file. --add-optionsPrepend the specified string, which may include whitespace, before any other options when invoking the virtual machine in the resulting image. --vendor-bug-url Override the vendor bug URL baked into the build. The value of the system property "java.vendor.url.bug" will be . --vendor-vm-bug-url Override the vendor VM bug URL baked into the build. The URL displayed in VM error logs will be . --vendor-version Override the vendor version string baked into the build, if any. The value of the system property "java.vendor.version" will be . For options requiring a , the value will be a comma separated list of elements each using one the following forms: glob: regex: @ where filename is the name of a file containing patterns to be used, one pattern per line - Commit messages: - Tidying up plugins Changes: https://git.openjdk.java.net/jdk/pull/305/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=305&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8218685 Stats: 233 lines in 25 files changed: 204 ins; 5 del; 24 mod Patch: https://git.openjdk.java.net/jdk/pull/305.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/305/head:pull/305 PR: https://git.openjdk.java.net/jdk/pull/305
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Mon, 21 Sep 2020 10:47:40 GMT, Hannes Wallnöfer wrote:
>> This pull request is identical with the RFR previously sent for the
>> Mercurial repository:
>>
>> https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html
>>
>> I'm copy-pasting the comments from the original RFR below.
>>
>> Most of the new code is added to the Extern class where it fits in quite
>> nicely and can use the existing supporting
>> code for setting up external links.
>> The default behaviour is to generate links to docs.oracle.com for released
>> versions and download.java.net for
>> prereleases. Platform documentation URLs can be configured using the new
>> --link-platform-properties option to
>> provide a properties file with URLs pointing to to alternative locations.
>> See the CSR linked above for more details on
>> the new options. The feature can be disabled using the --no-platform-link
>> option, generating the same output as
>> previously. One problem I had to solve was how to handle the transition
>> from prerelease versions to final releases,
>> since the location of the documentation changes in this transition. For
>> obvious reasons we don’t want to make that
>> switch via code change at a time shortly before release. The way it is done
>> is that we determine if the current
>> javadoc instance is a prerelease version as indicated by the Version
>> returned by BaseConfiguration#getDocletVersion(),
>> and then check whether the release/source version of the current javadoc
>> execution uses the same (latest) version. This
>> means that that only the latest version will ever generate prerelease links
>> (e.g. running current javadoc 16 with
>> source version 15 will generate links to the final release documentation)
>> but I think this is acceptable. Another
>> issue I spent some time getting right was tests. New releases require a new
>> element-list resource*), so tests have to
>> pick up new releases. On the other hand, we don’t want hundreds of tests to
>> fail when a new release is added, ideally
>> there should be one test with a clear message. Because of this, when a
>> release is encountered for which no
>> element-list is available a warning is generated instead of an error, and
>> the documentation is generated without
>> platform links as if running with --no-platform-link option. This allows
>> most existing tests to pass and just the new
>> test to fail with a relatively clear message of what is wrong.
>> *) I also thought about generating the element-list for the current release
>> at build time. It’s quite involved, and we
>> still need to maintain element-lists for older versions, so I’m not sure
>> it’s worth it.
>>
>> For existing tests that check output affected by the new option I added the
>> --no-platform-link option to disable the
>> feature. Otherwise we’d have to update those tests with each new release (or
>> else freeze the tests to use some static
>> release or source version, which we don’t want either). I updated the CSR
>> to the new code. It also needs to be
>> reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181
>>
>> Thanks,
>> Hannes
>
> Hannes Wallnöfer has updated the pull request incrementally with three
> additional commits since the last revision:
>
> - Merge pull request #1 from lahodaj/JDK-8216497
>
>Automatically generate the elements-list data from the ct.sym for releases
> 11+, including the current release under
>development
> - Generating current release list for javadoc; using hardcoded lists for
> versions < 11
> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
> historical data.
Generally excellent. Some feedback inline.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
line 350:
> 348:
> 349: doclet.usage.excludedocfilessubdir.parameters=\
> 350: :..
3 dots for ellipsis? 2 dots is "parent directory"
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
line 384:
> 382:
> 383: doclet.usage.no-platform-link.description=\
> 384: Do not generate links to platform documentation
Suggest:
Do not generate links to the platform documentation
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
line 194:
> 192:
> 193: /**
> 194: * Argument for command-line option {@code --no-platform-link}.
minor: would "--no-platform-links" (plural) be a better name for the option?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
line 435:
> 433: },
> 434:
> 435: new Option(resources, "--no-platform-link") {
Repeating preceding comment: would `--no-platform-links` (plural) be a better
name?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
line 236:
> 234: * @param linkPlatformProperties path or URL to properties fi
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Tue, 22 Sep 2020 17:24:19 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Merge pull request #1 from lahodaj/JDK-8216497
>>
>>Automatically generate the elements-list data from the ct.sym for
>> releases 11+, including the current release under
>>development
>> - Generating current release list for javadoc; using hardcoded lists for
>> versions < 11
>> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
>> historical data.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
> line 323:
>
>> 321: props.load(inputStream);
>> 322: }
>> 323: url = props.getProperty("doclet.platform.docs." + version);
>
> Similar to other file-or-url arguments: good!
As a possibly-later cleanup, should we have a single utility method somewhere
(in this class) to open a stream on a
file-or-url?
-
PR: https://git.openjdk.java.net/jdk/pull/171
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v3]
On Mon, 21 Sep 2020 07:10:47 GMT, Bernhard Urban-Forster wrote: >> I assume you need the rest of the PATH on Windows. > >> I assume you need the rest of the PATH on Windows. > > Doesn't look like it actually. I've reverted it, thanks for catching it. Thanks, I tried the updated patch and it works now. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8252847: Optimize primitive arrayCopy stubs using AVX-512 masked instructions [v3]
On Tue, 22 Sep 2020 03:34:37 GMT, Vladimir Kozlov wrote: > @jatin-bhateja Can you put summary of performance improvement into JBS? Yes, I have added the summary to JBS - PR: https://git.openjdk.java.net/jdk/pull/61
Re: RFR: 8252847: Optimize primitive arrayCopy stubs using AVX-512 masked instructions [v3]
On Tue, 22 Sep 2020 03:34:37 GMT, Vladimir Kozlov wrote: > @jatin-bhateja Can you put summary of performance improvement into JBS? yes, I have added the summary in JBS. - PR: https://git.openjdk.java.net/jdk/pull/61
Integrated: 8253208: Move CDS related code to a separate class
On Fri, 18 Sep 2020 23:47:56 GMT, Yumin Qi wrote: > With more CDS related code added to VM, it is time to move CDS code to a > separate class. CDS is the new class which is > specific to CDS. > Tests: tier1-4 This pull request has now been integrated. Changeset: c1df13b8 Author:Yumin Qi URL: https://git.openjdk.java.net/jdk/commit/c1df13b8 Stats: 218 lines in 23 files changed: 53 ins; 119 del; 46 mod 8253208: Move CDS related code to a separate class Reviewed-by: mchung, iklam - PR: https://git.openjdk.java.net/jdk/pull/261
Re: RFR: 8253208: Move CDS related code to a separate class [v3]
On Mon, 21 Sep 2020 22:24:15 GMT, Yumin Qi wrote: >> With more CDS related code added to VM, it is time to move CDS code to a >> separate class. CDS is the new class which is >> specific to CDS. >> Tests: tier1-4 > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > 8253208: Move CDS related code to a separate class Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/261
RFR: 8253476: TestUseContainerSupport.java fails on some Linux kernels w/o…
Please review this small change to remove "--memory 200m" option from TestUseContainerSupport.java. This can cause test failures on systems where swap accounting is disabled. - Commit messages: - 8253476: TestUseContainerSupport.java fails on some Linux kernels w/o swap limit capabilities Changes: https://git.openjdk.java.net/jdk/pull/303/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=303&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253476 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/303.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/303/head:pull/303 PR: https://git.openjdk.java.net/jdk/pull/303
Re: RFR: 8252847: New AVX512 optimized stubs for both conjoint and disjoint arraycopy [v5]
> Summary: > > 1) New AVX3 optimized stubs for both conjoint and disjoint arraycopy. > 2) Special instruction sequence blocks for copy sizes b/w 32-192 bytes. > 3) Block copy operation above 192 bytes is performed using destination > address aligned PRE-MAIN-POST loop. Main loop > copies 192 byte in one iteration and tail part fall over special instruction > sequence blocks. 4) Both small copy block > and aligned loop use 32 byte vector register to prevent and frequency penalty > for copy sizes less than AVX3Threshold. > 5) For block size above AVX3Theshold both special blocks and loop operate > using 64 byte register. 6) In case user > sets the maximum vector size to 32 bytes, forward copy (disjoint) operations > are done using efficient REP MOVS for copy > sizes above 4096 bytes. JMH Results: > System : CascadeLake Server, Intel(R) Xeon(R) Platinum 8280L CPU @ > 2.70GHz > Micros : test/micro/org/openjdk/bench/java/lang/ArrayCopy*.java > Baseline : > [http://cr.openjdk.java.net/~jbhateja/8252847/JMH_results/ArrayCopy_AVX3_Stubs_Baseline.txt]() > WithOpt : > [http://cr.openjdk.java.net/~jbhateja/8252847/JMH_results/ArrayCopy_AVX3_Stubs_WithOpts.txt]() Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision: 8252847 : Modifying file permission to resolve jcheck failure. - Changes: - all: https://git.openjdk.java.net/jdk/pull/61/files - new: https://git.openjdk.java.net/jdk/pull/61/files/fadd3687..78c4fe73 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=61&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=61&range=03-04 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/61.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/61/head:pull/61 PR: https://git.openjdk.java.net/jdk/pull/61
Re: RFR: 8252847: New AVX512 optimized stubs for both conjoint and disjoint arraycopy [v4]
> Summary: > > 1) New AVX3 optimized stubs for both conjoint and disjoint arraycopy. > 2) Special instruction sequence blocks for copy sizes b/w 32-192 bytes. > 3) Block copy operation above 192 bytes is performed using destination > address aligned PRE-MAIN-POST loop. Main loop > copies 192 byte in one iteration and tail part fall over special instruction > sequence blocks. 4) Both small copy block > and aligned loop use 32 byte vector register to prevent and frequency penalty > for copy sizes less than AVX3Threshold. > 5) For block size above AVX3Theshold both special blocks and loop operate > using 64 byte register. 6) In case user > sets the maximum vector size to 32 bytes, forward copy (disjoint) operations > are done using efficient REP MOVS for copy > sizes above 4096 bytes. JMH Results: > System : CascadeLake Server, Intel(R) Xeon(R) Platinum 8280L CPU @ > 2.70GHz > Micros : test/micro/org/openjdk/bench/java/lang/ArrayCopy*.java > Baseline : > [http://cr.openjdk.java.net/~jbhateja/8252847/JMH_results/ArrayCopy_AVX3_Stubs_Baseline.txt]() > WithOpt : > [http://cr.openjdk.java.net/~jbhateja/8252847/JMH_results/ArrayCopy_AVX3_Stubs_WithOpts.txt]() Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision: 8252847: Review comments resolution; code reorganized to cover arraycopy for reference types. - Changes: - all: https://git.openjdk.java.net/jdk/pull/61/files - new: https://git.openjdk.java.net/jdk/pull/61/files/271b6457..fadd3687 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=61&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=61&range=02-03 Stats: 691 lines in 5 files changed: 340 ins; 198 del; 153 mod Patch: https://git.openjdk.java.net/jdk/pull/61.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/61/head:pull/61 PR: https://git.openjdk.java.net/jdk/pull/61
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Mon, 21 Sep 2020 08:19:28 GMT, Alexey Bakhtin wrote:
> Hi,
>
> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support
> for Java GSS/Kerberos.
> Initial review is available at core-devs:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html
> This version removes "tls-unique" CB type from the list of possible channel
> binding types. The only supported type is
> "tls-server-end-point"
> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311
>
> Thank you
> Alexey
I'm mostly OK with the SASL/JGSS part, except for the small nits in this
comment. I'm not an expert on
HandshakeCompletedListener.
src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Client.java
line 156:
> 154: if (props != null) {
> 155: // TLS Channel Binding
> 156: byte[] tlsCB =
> (byte[])props.get("jdk.internal.sasl.tlschannelbinding");
You can say the name is defined in another class in another module. If we
really want to rename it one day we will know
where it's from.
src/java.security.jgss/share/classes/sun/security/jgss/krb5/InitialToken.java
line 389:
> 387: int acceptorAddressType = getAddrType(acceptorAddress,
> 388: (channelBinding instanceof TlsChannelBindingImpl)?
> 389:
> CHANNEL_BINDING_AF_UNSPEC:CHANNEL_BINDING_AF_NULL_ADDR);
Normally we put a white space around "?" and ":".
src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
line 82:
> 80: /**
> 81: * Parse value of "com.sun.jndi.ldap.tls.cbtype" property
> 82: * @param cbType
Please add a `@return` here, esp, about null.
src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
line 137:
> 135: public TlsChannelBindingType getType() {
> 136: return cbType;
> 137: }
Add a new line here.
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Tue, 22 Sep 2020 15:17:23 GMT, Alexey Bakhtin wrote:
>> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
>> line 63:
>>
>>> 61: * Channel binding on the basis of TLS Finished message
>>> 62: */
>>> 63: TLS_UNIQUE("tls-unique"),
>>
>> Is that still used? If not maybe it should be removed?
>
> No, It is not used.
> However, I'd like to leave it as is (it is mentioned in the documentation as
> unsupported value).
> Otherwise, TlsChannelBindingType enum will have one element only and should
> be simplified/removed in all places. In
> this case, it would be double work to add TlsChannelBindingType enum back in
> the future if "tls-unique" required. If
> required I can remove TLS_UNIQUE item, but not remove TlsChannelBindingType
> enum
OK
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char) [v3]
On Mon, 21 Sep 2020 12:45:55 GMT, Jason Tatton wrote: >> This is an implementation of the indexOf(char) intrinsic for StringLatin1 (1 >> byte encoded Strings). It is provided for >> x86 and ARM64. The implementation is greatly inspired by the indexOf(char) >> intrinsic for StringUTF16. To incorporate it >> I had to make a small change to StringLatin1.java (refactor of functionality >> to intrisified private method) as well as >> code for C2. Submitted to: hotspot-compiler-dev and core-libs-dev as this >> patch contains a change to hotspot and >> java/lang/StringLatin1.java https://bugs.openjdk.java.net/browse/JDK-8173585 >> >> Details of testing: >> >> I have created a jtreg test >> “compiler/intrinsics/string/TestStringLatin1IndexOfChar” to cover this new >> intrinsic. Note >> that, particularly for the x86 implementation of the intrinsic, the code >> path taken is dependent upon the length of the >> input String. Hence the test has been designed to cover all these cases. In >> summary they are: >> - A “short” string of < 16 characters. >> - A SIMD String of 16 – 31 characters. >> - A AVX2 SIMD String of 32 characters+. >> >> Hardware used for testing: >> - >> >> - Intel Xeon CPU E5-2680 (JVM did not recognize this as having AVX2 support) >> • Intel i7 processor (with AVX2 support). >> - AWS Graviton 2 (ARM 64 processor). >> >> I also ran; ‘run-test-tier1’ and ‘run-test-tier2’ for: x86_64 and aarch64. >> >> Possible future enhancements: >> >> For the x86 implementation there may be two further improvements we can make >> in order to improve performance of both >> the StringUTF16 and StringLatin1 indexOf(char) intrinsics: >> 1. Make use of AVX-512 instructions. >> 2. For “short” Strings (see below), I think it may be possible to modify the >> existing algorithm to still use SSE SIMD >> instructions instead of a loop. >> Benchmark results: >> >> **Without** the new StringLatin1 indexOf(char) intrinsic: >> >> | Benchmark | Mode | Cnt | Score | Error | Units | >> | - | - |- |- |- >> |- | >> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **26,389.129** | ± 182.581 >> | ns/op | >> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 17,885.383 | ± 435.933 | >> ns/op | >> >> >> **With** the new StringLatin1 indexOf(char) intrinsic: >> >> | Benchmark | Mode | Cnt | Score | Error | Units | >> | - | - |- |- |- >> |- | >> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **17,875.185** | ± 407.716 >> | ns/op | >> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 18,292.802 | ± 167.306 | >> ns/op | >> >> >> The objective of the patch is to bring the performance of StringLatin1 >> indexOf(char) in line with StringUTF16 >> indexOf(char) for x86 and ARM64. We can see above that this has been >> achieved. Similar results were obtained when >> running on ARM. > > Jason Tatton has updated the pull request incrementally with one additional > commit since the last revision: > > Add missing newline to end of vmSymbols.cpp src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 1889: > 1887: bind(SCAN_TO_32_CHAR_LOOP); > 1888: vmovdqu(vec3, Address(result, 0)); > 1889: vpcmpeqb(vec3, vec3, vec1, 1); Using `Assembler::AVX_256bit` instead of `1` will make this easier to understand. - PR: https://git.openjdk.java.net/jdk/pull/71
Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char) [v2]
On Fri, 18 Sep 2020 11:56:09 GMT, Jason Tatton
wrote:
>> This is an implementation of the indexOf(char) intrinsic for StringLatin1 (1
>> byte encoded Strings). It is provided for
>> x86 and ARM64. The implementation is greatly inspired by the indexOf(char)
>> intrinsic for StringUTF16. To incorporate it
>> I had to make a small change to StringLatin1.java (refactor of functionality
>> to intrisified private method) as well as
>> code for C2. Submitted to: hotspot-compiler-dev and core-libs-dev as this
>> patch contains a change to hotspot and
>> java/lang/StringLatin1.java https://bugs.openjdk.java.net/browse/JDK-8173585
>>
>> Details of testing:
>>
>> I have created a jtreg test
>> “compiler/intrinsics/string/TestStringLatin1IndexOfChar” to cover this new
>> intrinsic. Note
>> that, particularly for the x86 implementation of the intrinsic, the code
>> path taken is dependent upon the length of the
>> input String. Hence the test has been designed to cover all these cases. In
>> summary they are:
>> - A “short” string of < 16 characters.
>> - A SIMD String of 16 – 31 characters.
>> - A AVX2 SIMD String of 32 characters+.
>>
>> Hardware used for testing:
>> -
>>
>> - Intel Xeon CPU E5-2680 (JVM did not recognize this as having AVX2 support)
>> • Intel i7 processor (with AVX2 support).
>> - AWS Graviton 2 (ARM 64 processor).
>>
>> I also ran; ‘run-test-tier1’ and ‘run-test-tier2’ for: x86_64 and aarch64.
>>
>> Possible future enhancements:
>>
>> For the x86 implementation there may be two further improvements we can make
>> in order to improve performance of both
>> the StringUTF16 and StringLatin1 indexOf(char) intrinsics:
>> 1. Make use of AVX-512 instructions.
>> 2. For “short” Strings (see below), I think it may be possible to modify the
>> existing algorithm to still use SSE SIMD
>> instructions instead of a loop.
>> Benchmark results:
>>
>> **Without** the new StringLatin1 indexOf(char) intrinsic:
>>
>> | Benchmark | Mode | Cnt | Score | Error | Units |
>> | - | - |- |- |-
>> |- |
>> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **26,389.129** | ± 182.581
>> | ns/op |
>> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 17,885.383 | ± 435.933 |
>> ns/op |
>>
>>
>> **With** the new StringLatin1 indexOf(char) intrinsic:
>>
>> | Benchmark | Mode | Cnt | Score | Error | Units |
>> | - | - |- |- |-
>> |- |
>> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **17,875.185** | ± 407.716
>> | ns/op |
>> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 18,292.802 | ± 167.306 |
>> ns/op |
>>
>>
>> The objective of the patch is to bring the performance of StringLatin1
>> indexOf(char) in line with StringUTF16
>> indexOf(char) for x86 and ARM64. We can see above that this has been
>> achieved. Similar results were obtained when
>> running on ARM.
>
> Jason Tatton has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now
> contains four commits:
> - Merge master
> - 8173585: further whitespace changes required by jcheck
> - JDK-8173585 - whitespace changes required by jcheck
> - JDK-8173585
Hi Jason,
thanks for bringing String.indexOf() for latin strings up to date with the
Unicode version.
Your changes look good except a few minor issues I've commented on right in the
code.
I'd only like to ask you if you could possibly improve your test a little bit.
As far as I understand, your search text
is a consecutive sequence of "abc" characters, so you'll always find the
character your searching for within the next
three characters of the source text. This won't exercise the loops of your
intrinsic. Maybe you can also add some test
versions where the search character will be found beyond the first 32/64
characters after "fromIndex"?
test/hotspot/jtreg/compiler/intrinsics/string/TestStringLatin1IndexOfChar.java
line 24:
> 22:
> 23: public static void main(String[] args) throws Exception {
> 24: for (int i = 0; i < 100_0; ++i) {//repeat such that we enter into
> C2 code...
The placement of the underscore looks strange to me. I'd expect it to separate
thousands (like 1_000) if at all but not
sure if id use it for one thousand at all as that's really not such a big
number that it is hard to read..
Also, the Tier4InvocationThreshold is 5000 so I'm not sure youre reaching C2?
src/hotspot/share/classfile/vmSymbols.cpp line 295:
> 293: if (symbol == NULL) return NO_SID;
> 294: return find_sid(symbol);
> 295: }
I think it is common sense to have a newline at the end of a file.
test/hotspot/jtreg/compiler/intrinsics/string/TestStringLatin1IndexOfChar.java
line 84:
> 82: }
> 83:
> 84: }
Please put an EOL at the end of the file.
test/micro/org/openjdk/bench/java/lang/StringIndexOf
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Tue, 22 Sep 2020 14:47:35 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support
>> for Java GSS/Kerberos.
>> Initial review is available at core-devs:
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html
>> This version removes "tls-unique" CB type from the list of possible channel
>> binding types. The only supported type is
>> "tls-server-end-point"
>> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311
>>
>> Thank you
>> Alexey
>
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
> line 63:
>
>> 61: * Channel binding on the basis of TLS Finished message
>> 62: */
>> 63: TLS_UNIQUE("tls-unique"),
>
> Is that still used? If not maybe it should be removed?
No, It is not used.
However, I'd like to leave it as is (it is mentioned in the documentation as
unsupported value).
Otherwise, TlsChannelBindingType enum will have one element only and should be
simplified/removed in all places. In
this case, it would be double work to add TlsChannelBindingType enum back in
the future if "tls-unique" required. If
required I can remove TLS_UNIQUE item, but not remove TlsChannelBindingType enum
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Tue, 22 Sep 2020 14:41:57 GMT, Daniel Fuchs wrote: >> Hi, >> >> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support >> for Java GSS/Kerberos. >> Initial review is available at core-devs: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html >> This version removes "tls-unique" CB type from the list of possible channel >> binding types. The only supported type is >> "tls-server-end-point" >> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 >> >> Thank you >> Alexey > > src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java line 994: > >> 992: } >> 993: >> 994: private CompletableFuture tlsHandshakeCompleted = > > Should be `final`? Thank you. Agree. It should be final. - PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Mon, 21 Sep 2020 08:19:28 GMT, Alexey Bakhtin wrote:
> Hi,
>
> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support
> for Java GSS/Kerberos.
> Initial review is available at core-devs:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html
> This version removes "tls-unique" CB type from the list of possible channel
> binding types. The only supported type is
> "tls-server-end-point"
> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311
>
> Thank you
> Alexey
src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java line 994:
> 992: }
> 993:
> 994: private CompletableFuture tlsHandshakeCompleted =
Should be `final`?
src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
line 63:
> 61: * Channel binding on the basis of TLS Finished message
> 62: */
> 63: TLS_UNIQUE("tls-unique"),
Is that still used? If not maybe it should be removed?
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests [v3]
On Tue, 22 Sep 2020 02:49:50 GMT, Alexander Matveev wrote: > > > > How about testing of other jpackage command line options in two phase mode? > > Like "--name", "--version"? Any plans to > > add them? > > Plan was to file separate bugs for any additional options. I do not think we > should put everything in one issue. This > issue for desktop integration for additional launchers. Agree that separate bugs can be filed for additional options, can you file them ? I am testing in conjunction with https://github.com/openjdk/jdk/pull/271 - PR: https://git.openjdk.java.net/jdk/pull/263
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Mon, 21 Sep 2020 08:19:28 GMT, Alexey Bakhtin wrote: > Hi, > > Plaese review JDK-8245527 fix which implements LDAP Channel Binding support > for Java GSS/Kerberos. > Initial review is available at core-devs: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html > This version removes "tls-unique" CB type from the list of possible channel > binding types. The only supported type is > "tls-server-end-point" > CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 > > Thank you > Alexey Thanks for the PR Alexey! Let me run a last round of testing - and if that comes back clean I'll approve. Please don't integrate until you get a reviewer from security-libs too. best regards, -- daniel - PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8251188: Update LDAP tests not to use wildcard addresses
On Tue, 22 Sep 2020 12:30:47 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> Please help to review
>> [JDK-8251188](https://bugs.openjdk.java.net/browse/JDK-8251188) fix which
>> helps to improve LDAP
>> tests stability. The list of changes: 1. Usages of wildcard address have
>> been replaced with loopback address. This
>> change includes addition of `LDAPTestUtils.initEnv` method that takes LDAP
>> provider URL as a parameter. 2.
>> `DeadServerTimeoutSSLTest.java` was also updated to fix the intermittent
>> failures reported by [JDK-8152654
>> ](https://bugs.openjdk.java.net/browse/JDK-8152654) and
>> [JDK-8169942](https://bugs.openjdk.java.net/browse/JDK-8169942). Before the
>> fix the failure rate was 1 out of 4 runs.
>> After the fix it was executed 400+ times alongside to other LDAP tests, and
>> showed no failures, and therefore removed
>> from the problem list. Thank you, Aleksei
>
> test/jdk/com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java line 171:
>
>> 169: System.err.println("Server socket. Failure to accept
>> connection:");
>> 170: e.printStackTrace();
>> 171: }
>
> I wonder if removing the while (true) loop will make the test more
> susceptible of failing in timeout if the server ever
> receives a connection request from an unexpected client (we've seen that
> happening in the past with networking tests).
> Is there anyway the server could attempt to verify that the accepted socket
> is from the expected client, and close it
> and go back to accepting if it's not? Maybe by looking at the accepted socket
> remote address & port?
Thanks for the good suggestion Daniel. I will modify it to look at the remote
socket's address and port.
-
PR: https://git.openjdk.java.net/jdk/pull/252
Re: RFR: 8251188: Update LDAP tests not to use wildcard addresses
On Fri, 18 Sep 2020 12:59:07 GMT, Aleksei Efimov wrote:
> Hi,
>
> Please help to review
> [JDK-8251188](https://bugs.openjdk.java.net/browse/JDK-8251188) fix which
> helps to improve LDAP
> tests stability. The list of changes: 1. Usages of wildcard address have been
> replaced with loopback address. This
> change includes addition of `LDAPTestUtils.initEnv` method that takes LDAP
> provider URL as a parameter. 2.
> `DeadServerTimeoutSSLTest.java` was also updated to fix the intermittent
> failures reported by [JDK-8152654
> ](https://bugs.openjdk.java.net/browse/JDK-8152654) and
> [JDK-8169942](https://bugs.openjdk.java.net/browse/JDK-8169942). Before the
> fix the failure rate was 1 out of 4 runs.
> After the fix it was executed 400+ times alongside to other LDAP tests, and
> showed no failures, and therefore removed
> from the problem list. Thank you, Aleksei
Very good and thorough fix Aleksei! This looks mostly very good to me - and I
have only one doubt, which I commented on
in the code.
test/jdk/com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java line 171:
> 169: System.err.println("Server socket. Failure to accept
> connection:");
> 170: e.printStackTrace();
> 171: }
I wonder if removing the while (true) loop will make the test more susceptible
of failing in timeout if the server ever
receives a connection request from an unexpected client (we've seen that
happening in the past with networking tests).
Is there anyway the server could attempt to verify that the accepted socket is
from the expected client, and close it
and go back to accepting if it's not? Maybe by looking at the accepted socket
remote address & port?
-
PR: https://git.openjdk.java.net/jdk/pull/252
Re: RFR: 8253149: Building an installer from invalid app image fails on Window… [v3]
> 8253149: Building an installer from invalid app image fails on Windows and > Linux > When jpackage builds a package from an app-image that was not generated by > jpackage, the tool should give user a > warning message, and then complete the package anyway. Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: 8253149: Building an installer from invalid app image fails (revision 2) - Changes: - all: https://git.openjdk.java.net/jdk/pull/271/files - new: https://git.openjdk.java.net/jdk/pull/271/files/e3f8a8fa..48001605 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=271&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=271&range=01-02 Stats: 5 lines in 5 files changed: 0 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/271.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/271/head:pull/271 PR: https://git.openjdk.java.net/jdk/pull/271
Math.exp() yields different results on 32 bit systems
Hi!
According to a bug report at AdoptOpenJDK [1] Math.exp(10_000.0) yields
different results on 32 bit systems than on 64 bit systems.
public class Test {
public static void main(String[] args) {
System.out.println(Math.exp(10_000.0));
}
}
On 64 bit systems the code above prints Infinity. On 32 bit systems the result
is 0.0. I think that's wrong, but I'm far from an expert in floating point
arithmetics.
Affected versions:
* OpenJDK 9.0.1
* OpenJDK 11.0.8
* OpenJDK 15
Not affected:
* OpenJDK 8u265
We tested both with AdoptOpenJDK and Azul Zulu on Windows and Linux on x86 and
x64. If we disable intrinsics (-XX:+UnlockDiagnosticVMOptions
-XX:-UseLibmIntrinsic), Infinity is printed on x86 systems, too.
Is this expected behavior?
Best,
Andreas
[1] https://github.com/AdoptOpenJDK/openjdk-support/issues/182
Re: RFR: 8246774: Record Classes (final) implementation
On Mon, 21 Sep 2020 23:21:18 GMT, Vicente Romero wrote: >> Hi Vicente, >> Please file a separate subtask for the javax.lang.model changes. This helps >> with the JSR 269 MR paperwork. >> Thanks, >> -Joe > > note: I have removed from the original patch the code related to > javax.lang.model, I will publish them in a separate PR @vicente-romero-oracle I noticed that we can also remove the preview args from the record serialization tests and ObjectMethodsTest. I opened a PR against the branch in your fork. You should be able to just merge in the changes. See https://github.com/vicente-romero-oracle/jdk/pull/1 - PR: https://git.openjdk.java.net/jdk/pull/290
Re: RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests [v3]
> https://bugs.openjdk.java.net/browse/JDK-8231591 > > - Added MultiLauncherTwoPhaseTest which uses predefine app image with > multiple launcher and tests to make sure installer > will create shortcuts for all launchers. > - Fixed Linux DesktopIntegration to create shortcuts for additional launcher > if we using pre-define app image. Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision: 8231591: [TESTBUG] Create additional two phase jpackage tests (revision 2) - Changes: - all: https://git.openjdk.java.net/jdk/pull/263/files - new: https://git.openjdk.java.net/jdk/pull/263/files/5303fcc0..650cf21e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=263&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=263&range=01-02 Stats: 20 lines in 1 file changed: 10 ins; 9 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/263.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/263/head:pull/263 PR: https://git.openjdk.java.net/jdk/pull/263
Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char)
On Tue, 22 Sep 2020 02:31:14 GMT, Vladimir Kozlov wrote: >>> >Can you explain where this restricted effect is documented? >> >>> Certainly! I’ve found that determining the capability of the CPU and >>> whether to enable AVX2 support if the chip >>> supports it is mostly controlled in: vm_version_x86.cpp specifically: >>> get_processor_features and in >>> generate_get_cpu_info. >> >> Yes, I can see what the code does. I was asking where the cpu behaviour is >> documented independent of the code. >> >>> In order to test the patch comprehensively I had to track down an Intel >>> Core i7 (I7-9750H) processor which the >>> aforementioned code permitted AVX2 instructions for (maybe this is an error >>> and it should not be enabled for this >>> processor though) as most of the infrastructure I personally use here at >>> AWS runs on Intel Xeon processors - I also >>> tested on a E5-2680 which the JVM does not enable AVX2 for. >> >> 'maybe'? The documentation Andrew provided mentioned Xeon E5 v3 which I >> believe is a Skylake design. However, the code >> I pointed you at in vm_version_x86 which claims to detect 'early Skylake' >> designs is only disabling AVX512 support. It >> still enables AVX2. Similarly, the code that generates machine code to check >> the processor capabilities has a special >> check if use_evex is set (i.e. AVX3 is requested) which disables AVX512 for >> Skylake but does not disable AVX2 support. >>> However, this is just the Intel side of things. When it comes to AMD I read >>> that the AMD Zen 2 architecture, of which >>> the current flagship: Threadripper 3990X, is based, is able to support AVX2 >>> without the frequency scaling which >>> some/all(?) of the Intel chips incur. I personally don’t have access to one >>> of these chips so I cannot confirm how it >>> is classified in the JVM. >> >> Well, it would be good to know where you read that and to see if that >> confirms thar the code is avoiding the issue >> Andrew raised. >>> Also, I found when investigating this that there is actually a JVM flag >>> which can be used to control what level of AVX >>> is enabled: -XX:UseAVX=version. >> >> Yes, indeed. However, what I am trying to understand is whether the current >> code is bypassing the problem Andrew >> brought up in the cases where that problem actually exists. It doesn't look >> like it so far given that the problem >> applies to AVX2 and only AVX512 support is being disabled and, even then >> only for some (Skylake) processors. Without >> some clear documentation of what processors suffer from this power surge >> problem it will not be possible to decide >> whether this patch is doing the right thing or not. > > Based on comment by @jatin-bhateja (Intel) frequency level switchover pointed > by @theRealAph is sensitive to vector > size https://github.com/openjdk/jdk/pull/144#issuecomment-692044896 > > By keeping vector size less or equal to 32 bytes we should avoid it. And as I > can see this intrinsic code is using 32 > bytes (chars) and 16 bytes vectors: `pbroadcastb(vec1, vec1, > Assembler::AVX_256bit);` > Also we never had issues with AVX2. only with AVX512 regarding performance > hit: > https://bugs.openjdk.java.net/browse/JDK-8221092 > > I would like to see performance numbers for for all values of UseAVX flag : > 0, 1, 2, 3 > > The usage is guarded UseSSE42Intrinsics in UseSSE42Intrinsics predicate in > .ad file. Make sure to test with UseAVX=0 to > make sure that some avx instructions are not mixed into non avx code. And > also with UseSSE=2 (for example) to make sure > shared code correctly recognize that intrinsics is not supported. @vnkozlov @mknjc @jatin-bhateja Thanks for providing the relevant details. I'm now quite content that this patch avoids any potential frequency scaling problem. I'm also glad that an explanation of why this is so is now available -- although it's not perfect that we are relying on a stackoverflow post for the full details. - PR: https://git.openjdk.java.net/jdk/pull/71
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Sat, 12 Sep 2020 00:19:00 GMT, Vladimir Kozlov wrote: >> Philippe Marschall has refreshed the contents of this pull request, and >> previous commits have been removed. The >> incremental views will show differences compared to the previous content of >> the PR. The pull request contains one new >> commit since the last revision: >> 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate > > Marked as reviewed by kvn (Reviewer). Have you run the JFR tests in test/jdk/jdk/jfr? - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v4]
On Mon, 21 Sep 2020 19:06:44 GMT, Lance Andersen wrote: >> Hi all, >> >> Please review the fix for JDK-8252739 which addresses an issue introduced by >> https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored >> the offset specified by >> Deflater.setDictionary. Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly >> as well as the java/util/zip and >> java/util/jar JCK tests. > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > More updates to testByteBufferDirect in DeflaterDictionaryTests.java Given that the offset handling is buggy then I think the test should at least cover the cases where the offset is 0 or out of bounds. No problem separating out further test coverage for the other setDictionary methods into a separate issue. - PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v11]
On Tue, 22 Sep 2020 01:25:16 GMT, Ian Graves wrote:
>> Related to [JDK-8252730 jlink does not create reproducible builds on
>> different
>> servers](https://bugs.openjdk.java.net/browse/JDK-8252730). Introduces
>> ordering based on `Archive` module names to
>> ensure stable files (and file signatures) across generated JDK images by
>> jlink.
>
> Ian Graves has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Cleanup on Test 3
Marked as reviewed by alanb (Reviewer).
test/jdk/tools/jlink/JLinkReproducible3Test.java line 67:
> 65:
> 66: Path copiedJlink1 = Optional.of(
> 67: Paths.get(copyJdk1Dir.toString(), "bin", "jlink"))
Could use copyJDk1Dir.resolve("bin").resolve("jlink") here to avoid going to
strings and back.
-
PR: https://git.openjdk.java.net/jdk/pull/156
Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v9]
On Tue, 22 Sep 2020 01:22:03 GMT, Ian Graves wrote: >> Whatever is easiest but 2 x copy tree would be simpler and the result should >> be two identical JDKs as reported in >> JDK-8252730. > > I misunderstood what you meant when I read it earlier today, apologies! The > new changes should address these enumerated > issues. The updated test looks okay, I can sponsor if you need it. - PR: https://git.openjdk.java.net/jdk/pull/156
