Re: jpackage usage problems
Hi there. I have another case where running jpackage emits Bundler DEB Bundle failed because of java.lang.NullPointerException: Cannot invoke "java.lang.CharSequence.length()" because "this.text" is null I have never experienced NullPointerExceptions in other JDK tools. Seems to me JPackage is a new kid on the block. In this case, JPackage is invoked to finish from an existing appimage to the final package. the failure is caused because some important files are missing in appimage. The previous run is even more puzzling for me. It is supposed to create the appimage directory. And while it emits no error message, on my development machine it runs reliably while on the Github Runner it just does not create all files - no error message, no exit code - nothing. Any idea what I could be looking at? Hiran On Mon, 2022-04-04 at 08:49 +0200, Hiran Chaudhuri wrote: > Hello Alex, > > I tried running the same command with the jdk19 you pointed at. As a > first, I get this output: > > $JAVA_HOME/bin/jpackage --version > 19-ea > > With that, I invoked the command as before, and i fairly got the same > output. So the behaviour in the latest version has not changed. > > Two things I'd like to point out: > - there is no JPackage version in the output itself so both you and > me > have to trust the correct one was called > - I found out the mistake: I should have used > --app-image build/app-image/SettlersRemake > but JPackage gave a warning in the beginning and processed all the > rest > > Hiran > > > $JAVA_HOME/bin/jpackage --app-image build/app-image --verbose --dest > build > [08:41:58.675] Warning: app-image dir not generated by jpackage. > [08:41:58.688] Running dpkg > [08:41:58.696] Command [PID: 21712]: > dpkg --print-architecture > [08:41:58.696] Output: > amd64 > [08:41:58.698] Returned: 0 > > [08:41:58.702] Running dpkg > [08:41:58.719] Command [PID: 21714]: > dpkg -s coreutils > [08:41:58.719] Output: > Package: coreutils > Essential: yes > Status: install ok installed > Priority: required > Section: utils > Installed-Size: 7196 > Maintainer: Ubuntu Developers < > ubuntu-devel-disc...@lists.ubuntu.com> > Architecture: amd64 > Multi-Arch: foreign > Version: 8.30-3ubuntu2 > Pre-Depends: libacl1 (>= 2.2.23), libattr1 (>= 1:2.4.44), libc6 > (>= > 2.28), libselinux1 (>= 2.1.13) > Description: GNU core utilities > This package contains the basic file, shell and text > manipulation > utilities which are expected to exist on every operating system. > . > Specifically, this package includes: > arch base64 basename cat chcon chgrp chmod chown chroot cksum > comm > cp > csplit cut date dd df dir dircolors dirname du echo env expand > expr > factor false flock fmt fold groups head hostid id install join > link ln > logname ls md5sum mkdir mkfifo mknod mktemp mv nice nl nohup > nproc > numfmt > od paste pathchk pinky pr printenv printf ptx pwd readlink > realpath rm > rmdir runcon sha*sum seq shred sleep sort split stat stty sum > sync > tac > tail tee test timeout touch tr true truncate tsort tty uname > unexpand > uniq unlink users vdir wc who whoami yes > Homepage: http://gnu.org/software/coreutils > Original-Maintainer: Michael Stone > [08:41:58.719] Returned: 0 > > [08:41:58.720] Running dpkg-deb > [08:41:58.722] Warning: app-image dir not generated by jpackage. > [08:41:58.723] Warning: app-image dir not generated by jpackage. > [08:41:58.723] java.lang.NullPointerException: Cannot invoke > "java.lang.CharSequence.length()" because "this.text" is null > at > java.base/java.util.regex.Matcher.getTextLength(Matcher.java:1769) > at java.base/java.util.regex.Matcher.reset(Matcher.java:415) > at java.base/java.util.regex.Matcher.(Matcher.java:252) > at > java.base/java.util.regex.Pattern.matcher(Pattern.java:1144) > at > jdk.jpackage/jdk.jpackage.internal.LinuxDebBundler.lambda$static$1(Li > nu > xDebBundler.java:84) > at > jdk.jpackage/jdk.jpackage.internal.LinuxPackageBundler.validate(Linux > Pa > ckageBundler.java:72) > at > jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments > .j > ava:688) > at > jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Argumen > ts > .java:561) > at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:91) > at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:52) > [08:41:58.725] jdk.jpackage.internal.PackagerException: Bundler DEB > Bundle failed because of java.lang.NullPointerException: Cannot > invoke > "java.lang.CharSequence.length()" because "this.text" is null > at > jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments > .j > ava:710) > at > jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Argumen > ts > .java:561) > at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:91) >
Re: RFR: JDK-8281006 Module::getResourceAsStream should check if the resource is open unconditionally when caller is null [v2]
On Thu, 7 Apr 2022 00:56:42 GMT, Tim Prinzing wrote:
>> Created a test called NullCallerGetResource to test
>> Module::getResourceAsStream and Class::getResourceAsStream from the native
>> level.
>>
>> At the java level the test builds a test module called 'n' which opens the
>> package 'open' to everyone. There is also a package 'closed' which is
>> neither opened or exported. Both packages have a text file called
>> 'test.txt'. The open package has a class called OpenResources and the closed
>> package has a class called ClosedResources. The native test is launched with
>> the test module n added.
>>
>> At the native level the test tries to read both the open and closed resource
>> from both the classes and the module. It performs the equivalent of the
>> following java operations:
>>
>> Class c = open.OpenResources.fetchClass();
>> InputStream in = c.getResourceAsStream("test.txt");
>> assert(in != null); in.close();
>>
>> Module n = c.getModule();
>> in = n.getResourceAsStream("open/test.txt");
>> assert(in != null); in.close();
>>
>> Class closed = closed.ClosedResources.fetchClass();
>> assert(closedsStream("test.txt") == null);
>> assert(n.getResourceAsStream("closed/test.txt") == null);
>>
>> The test initially threw an NPE when trying to fetch the open resource. The
>> Module class was fixed by removing the fragment with the (caller == null)
>> test in getResourceAsStream, and changing the call to isOpen(String,Module)
>> to use EVERYONE_MODULE if the caller module is null.
>
> Tim Prinzing has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update copyright date
Tim - this creates a conflict between the spec and implementation, the changes
to the 2-arg isOpen method need to be dropped so that it continues to throw NPE
if module is null.
-
Changes requested by alanb (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8134
Re: RFR: 8265768 [aarch64] Use glibc libm impl for dlog,dlog10,dexp iff 2.29 or greater on AArch64.
On Fri, 1 Apr 2022 15:38:36 GMT, Andrew Haley wrote: >> Will this patch change `java.lang.Math`, `java.lang.StrictMath` or both? >> I've noticed differences in iterative machine learning algorithms using exp >> & log across different JVMs and architectures which we try to track in >> [Tribuo](https://github.com/oracle/tribuo) by recording the JVM & arch in >> our model provenance objects. If this patch is integrated will there be an >> easy way to get (e.g. from `System.getProperty`) what implementation of exp >> is in use by the current JVM? Otherwise I won't be able to notify users that >> the model may not reproduce if they rerun the same computation on different >> versions of Linux with the same JVM & architecture. > >> Will this patch change `java.lang.Math`, `java.lang.StrictMath` or both? >> I've noticed differences in iterative machine learning algorithms using exp >> & log across different JVMs and architectures which we try to track in >> [Tribuo](https://github.com/oracle/tribuo) by recording the JVM & arch in >> our model provenance objects. > > Exactly so, and that is why this patch was never integrated. This was only > ever going to be about `java.lang.Math`, but we foundered on the rock of > monotonicity. Here's the spec: > > "most methods with more than 0.5 ulp errors are required to be > semi-monotonic: whenever the mathematical function is non-decreasing, so is > the floating-point approximation, likewise, whenever the mathematical > function is non-increasing, so is the floating-point approximation. Not all > approximations that have 1 ulp accuracy will automatically meet the > monotonicity requirements." > > We couldn't guarantee we'd meet the monotonicity requirements if we used > glibc libm, so this patch was, with some regret, abandoned. @theRealAph Thanks for the summary. I closed the JBS issue as Won't Fix. - PR: https://git.openjdk.java.net/jdk/pull/3510
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Thu, 7 Apr 2022 01:16:32 GMT, Naoto Sato wrote:
>> test/jdk/java/text/Format/NumberFormat/CurrencyFormat.java line 63:
>>
>>> 61: Locale.of("it", "IT", "EURO"),
>>> 62: Locale.forLanguageTag("de-AT"),
>>> 63: Locale.forLanguageTag("fr-CH"),
>>
>> Use the new factory? Ok not to change as these are tests and there are too
>> many of them. It's not deprecated anyways.
>
> `Locale.forLanguageTag()` is a preferred way to create a `Locale`, as it
> validates the input language tag, while `Locale.of()` doesn't as well as
> Locale constructors.
Ok, I didn't realize the existing method is preferred over the new method in
creating a Locale. The javadoc does state that it does not make any syntactic
checks. That's good to know.
-
PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote: > This is a follow-on task after deprecating the Locale constructors > (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are > simple replacements to Locale constructors with `Locale.of()` or Locale > constants, such as `Locale.US`. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Thu, 7 Apr 2022 01:16:27 GMT, Naoto Sato wrote:
>> test/jdk/java/text/Format/DateFormat/DateFormatRoundTripTest.java line 81:
>>
>>> 79:
>>> 80: /**
>>> 81: * Parse a name like "fr_FR" into Locale.of("fr", "FR", "");
>>
>> Locale.France?
>
> The test code parses the input string (eg. "fr_FR") into 3 elements, `name`,
> `country`, and `variant`, then create a `Locale` using those 3 elements.
> Changing it to `Locale.FRANCE` does not seem right here.
I see.
-
PR: https://git.openjdk.java.net/jdk/pull/8130
RFR: 8284361: Updating ASM to 9.3 for JDK 19
We recently updated our ASM version to 9.2 and just this week version 9.3 was announced with support for JDK19 so it makes sense to update to this last version. Thanks in advance for the reviews, Vicente - Commit messages: - updating test ValidateJarWithSealedAndRecord - adaptations after automatic conversion - 8284361: Updating ASM to 9.3 for JDK 19 Changes: https://git.openjdk.java.net/jdk/pull/8135/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8135&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284361 Stats: 438 lines in 57 files changed: 73 ins; 70 del; 295 mod Patch: https://git.openjdk.java.net/jdk/pull/8135.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8135/head:pull/8135 PR: https://git.openjdk.java.net/jdk/pull/8135
Re: RFR: 8284036: Make ConcurrentHashMap.CollectionView a sealed hierarchy
On Mon, 4 Apr 2022 06:55:10 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which now marks `CollectionView` as > a `sealed` class with only `EntrySetView`, `KeySetView` and `ValuesView` as > the sub-classes? This change corresponds to > https://bugs.openjdk.java.net/browse/JDK-8284036. > > A CSR has also been drafted for this change > https://bugs.openjdk.java.net/browse/JDK-8284272. As noted in the CSR, > marking this class as `sealed` and marking `KeySetView` as `final` shouldn't > have any impact in general and also in context of > serialization/de-serialization. > > tier1, tier2, tier3 tests have been run successfully with this change. Making KeySetView final (CollectionView a sealed hierarchy) expresses design intent more explicitly than having only private constructors. It also prevents the JVM from loading subclasses that somebody might contrive despite the inability to instantiate them. - PR: https://git.openjdk.java.net/jdk/pull/8085
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Thu, 7 Apr 2022 00:09:58 GMT, Joe Wang wrote:
>> This is a follow-on task after deprecating the Locale constructors
>> (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are
>> simple replacements to Locale constructors with `Locale.of()` or Locale
>> constants, such as `Locale.US`.
>
> test/jdk/java/text/Format/DateFormat/DateFormatRoundTripTest.java line 81:
>
>> 79:
>> 80: /**
>> 81: * Parse a name like "fr_FR" into Locale.of("fr", "FR", "");
>
> Locale.France?
The test code parses the input string (eg. "fr_FR") into 3 elements, `name`,
`country`, and `variant`, then create a `Locale` using those 3 elements.
Changing it to `Locale.FRANCE` does not seem right here.
> test/jdk/java/text/Format/NumberFormat/CurrencyFormat.java line 63:
>
>> 61: Locale.of("it", "IT", "EURO"),
>> 62: Locale.forLanguageTag("de-AT"),
>> 63: Locale.forLanguageTag("fr-CH"),
>
> Use the new factory? Ok not to change as these are tests and there are too
> many of them. It's not deprecated anyways.
`Locale.forLanguageTag()` is a preferred way to create a `Locale`, as it
validates the input language tag, while `Locale.of()` doesn't as well as Locale
constructors.
> test/jdk/java/text/Format/common/Bug6215962.java line 58:
>
>> 56: check(mf1, mf2, false);
>> 57:
>> 58: mf1 = new MessageFormat("{0}", Locale.of("ja", "JP"));
>
> Locale.JAPAN?
The test intends to compare the generated `MessageFormat` objects (`mf2` &
`mf1`) from using a constant `Locale.JAPAN` and the factory method. So I
believe leaving it as `Locale.of()` makes sense.
-
PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: JDK-8281006 Module::getResourceAsStream should check if the resource is open unconditionally when caller is null [v2]
> Created a test called NullCallerGetResource to test
> Module::getResourceAsStream and Class::getResourceAsStream from the native
> level.
>
> At the java level the test builds a test module called 'n' which opens the
> package 'open' to everyone. There is also a package 'closed' which is neither
> opened or exported. Both packages have a text file called 'test.txt'. The
> open package has a class called OpenResources and the closed package has a
> class called ClosedResources. The native test is launched with the test
> module n added.
>
> At the native level the test tries to read both the open and closed resource
> from both the classes and the module. It performs the equivalent of the
> following java operations:
>
> Class c = open.OpenResources.fetchClass();
> InputStream in = c.getResourceAsStream("test.txt");
> assert(in != null); in.close();
>
> Module n = c.getModule();
> in = n.getResourceAsStream("open/test.txt");
> assert(in != null); in.close();
>
> Class closed = closed.ClosedResources.fetchClass();
> assert(closedsStream("test.txt") == null);
> assert(n.getResourceAsStream("closed/test.txt") == null);
>
> The test initially threw an NPE when trying to fetch the open resource. The
> Module class was fixed by removing the fragment with the (caller == null)
> test in getResourceAsStream, and changing the call to isOpen(String,Module)
> to use EVERYONE_MODULE if the caller module is null.
Tim Prinzing has updated the pull request incrementally with one additional
commit since the last revision:
update copyright date
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8134/files
- new: https://git.openjdk.java.net/jdk/pull/8134/files/b702cae4..4b344e4d
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8134&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8134&range=00-01
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/8134.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8134/head:pull/8134
PR: https://git.openjdk.java.net/jdk/pull/8134
RFR: JDK-8281006 Module::getResourceAsStream should check if the resource is open unconditionally when caller is null
Created a test called NullCallerGetResource to test Module::getResourceAsStream
and Class::getResourceAsStream from the native level.
At the java level the test builds a test module called 'n' which opens the
package 'open' to everyone. There is also a package 'closed' which is neither
opened or exported. Both packages have a text file called 'test.txt'. The open
package has a class called OpenResources and the closed package has a class
called ClosedResources. The native test is launched with the test module n
added.
At the native level the test tries to read both the open and closed resource
from both the classes and the module. It performs the equivalent of the
following java operations:
Class c = open.OpenResources.fetchClass();
InputStream in = c.getResourceAsStream("test.txt");
assert(in != null); in.close();
Module n = c.getModule();
in = n.getResourceAsStream("open/test.txt");
assert(in != null); in.close();
Class closed = closed.ClosedResources.fetchClass();
assert(closedsStream("test.txt") == null);
assert(n.getResourceAsStream("closed/test.txt") == null);
The test initially threw an NPE when trying to fetch the open resource. The
Module class was fixed by removing the fragment with the (caller == null) test
in getResourceAsStream, and changing the call to isOpen(String,Module) to use
EVERYONE_MODULE if the caller module is null.
-
Commit messages:
- JDK-8281006 Module::getResourceAsStream should check if the resource is open
unconditionally when caller is null
Changes: https://git.openjdk.java.net/jdk/pull/8134/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8134&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8281006
Stats: 397 lines in 7 files changed: 391 ins; 5 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/8134.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8134/head:pull/8134
PR: https://git.openjdk.java.net/jdk/pull/8134
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote:
> This is a follow-on task after deprecating the Locale constructors
> (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are
> simple replacements to Locale constructors with `Locale.of()` or Locale
> constants, such as `Locale.US`.
test/jdk/java/text/Format/NumberFormat/CurrencyFormat.java line 63:
> 61: Locale.of("it", "IT", "EURO"),
> 62: Locale.forLanguageTag("de-AT"),
> 63: Locale.forLanguageTag("fr-CH"),
Use the new factory? Ok not to change as these are tests and there are too many
of them. It's not deprecated anyways.
test/jdk/java/text/Format/common/Bug6215962.java line 58:
> 56: check(mf1, mf2, false);
> 57:
> 58: mf1 = new MessageFormat("{0}", Locale.of("ja", "JP"));
Locale.JAPAN?
-
PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote:
> This is a follow-on task after deprecating the Locale constructors
> (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are
> simple replacements to Locale constructors with `Locale.of()` or Locale
> constants, such as `Locale.US`.
test/jdk/java/text/Format/DateFormat/NonGregorianFormatTest.java line 131:
> 129:
> 130: // Tests with the Japanese imperial calendar
> 131: Locale calendarLocale = Locale.of("ja", "JP", "JP");
Locale.JAPAN?
-
PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote:
> This is a follow-on task after deprecating the Locale constructors
> (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are
> simple replacements to Locale constructors with `Locale.of()` or Locale
> constants, such as `Locale.US`.
test/jdk/java/text/Format/DateFormat/DateFormatRoundTripTest.java line 81:
> 79:
> 80: /**
> 81: * Parse a name like "fr_FR" into Locale.of("fr", "FR", "");
Locale.France?
-
PR: https://git.openjdk.java.net/jdk/pull/8130
Re: fast way to infer caller
- Original Message -
> From: "Ceki Gülcü"
> To: "core-libs-dev"
> Sent: Thursday, April 7, 2022 1:04:11 AM
> Subject: Re: fast way to infer caller
> Hi Jason,
>
> Yes, the code was mentioned in SLF4J PR 271.
>
> The benchmark can be found at
>
> https://github.com/ceki/logback-perf/tree/master/src/main/java/ch/qos/logback/perf/caller
>
> Here is the gist of the StackWalker variant:
>
> public class CallerCompute {
>
>static private StackWalker WALKER = StackWalker.getInstance();
>
>static public String getCallerClass(int depth) {
>
>List frames = WALKER.walk(s -> s.limit(4).toList());
> process the returned frames
>}
> }
>
> The above executes at around 1.8 microseconds per call whereas the code
> in LogRecord.getSourceClassName() executes at 5 microseconds per call.
> The cost of LogRecord constructor is negligible at around 25 nanoseconds.
>
> Anyway, thank you for the suggestion.
You can declare WALKER "final", it should help. This is also better to call
getCallerClass ASAP and then pass the class to the other stack frames instead
of having to skip those stack frames when calling getCallerClass. You may also
try able to improve the speed by doing a == before calling equals(String) if
you call String.intern() on THIS_CLASS_NAME.
>
> --
> Ceki
Rémi
>
>
> On 4/7/2022 12:27 AM, Jason Mehrens wrote:
>> Ceki,
>>
>> Looks like the benchmark code is from
>> https://github.com/qos-ch/slf4j/pull/271
>>
>> Looks like the benchmark code is doing a collection so perhaps that is some
>> of
>> the performance hit?
>> Have you benchmarked java.util.LogRecord.getSourceClassName() to compare with
>> your StackWalker benchmark?
>>
>> https://github.com/openjdk/jdk/blob/master/src/java.logging/share/classes/java/util/logging/LogRecord.java#L754
>>
>> Jason
>>
>>
>> From: core-libs-dev on behalf of Ceki
>> Gülcü
>> Sent: Wednesday, April 6, 2022 4:26 PM
>> To: core-libs-dev
>> Subject: Re: fast way to infer caller
>>
>>
>> Hi Rémi,
>>
>> Thank you for your answer.
>>
>> According to some benchmarks on a i7-8565U Intel CPU (quite average
>> CPU), here are the costs of computing the caller class via different
>> methods:
>>
>> using new Throwable().getStackTrace: 11 microseconds per call
>> using StackWalker API: 1.8 microseconds per call
>> using SecurityManager: 0.9 microseconds per call
>>
>> While a six fold improvement (StackWalker compared to new Thorowable) is
>> nothing to sneeze at, the performance of StackWalker is not as good as
>> with a custom SecurityManager.
>>
>> I have not said so explicitly but the aim here is to allow the user to
>> obtain a new logger by calling LoggerFactory.getLogger() with no
>> arguments with the returned logger named after the caller class.
>>
>> Spending 1 or 2 microseconds for this call is OK if the logger is a
>> static field. However, if the logger is an instance field, then spending
>> 2 microseconds per host object instance just to obtain a logger might
>> have a noticeable impact on performance. It follows that the performance
>> of LoggerFactory.getLogger() must be exceptionally good, assuming we
>> wish to avoid having the user accidentally shooting herself on the foot,
>> ergo the 100 nanosecond performance per call requirement.
>>
>> Noting that invoking MethodHandles.lookup().lookupClass() seems very
>> fast (about 2 nanoseconds), I would be very interested if new
>> lookup(int index) method were added to java.lang.invoke.MethodHandles
>> class, with index designating the desired index on the call stack.
>>
>> Does the above make sense? How difficult would it be to add such a method?
>>
>> --
>> Ceki Gülcü
>>
>>
>> On 4/6/2022 5:52 PM, Remi Forax wrote:
>>> - Original Message -
From: "Ceki Gülcü"
To: "core-libs-dev"
Sent: Wednesday, April 6, 2022 5:30:51 PM
Subject: fast way to infer caller
>>>
Hello,
>>>
>>> Hello,
>>>
As you are probably aware, one of the important primitives used in
logging libraries is inferring the caller of a given logging statement.
The current common practice is to create a throwable and process its
stack trace. This is rather wasteful and rather slow. As an alternative,
I have tried using the StackWalker API to infer the caller but was
unsatisfied with the performance.
MethodHandles.lookup().lookupClass() looks very promising except that
there is no way to specify the depth.
I am looking for a method to obtain the Nth caller at a cost of around
100 to 200 nanoseconds of CPU time. Do you think the JDK could cater
for this use case?
>>>
>>> We have designed the StackWalker with that in mind
>>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html
>>>
>>> see the discussion on StackWalker.getCallerClass()
>>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Stac
Re: fast way to infer caller
> On Apr 6, 2022, at 3:54 PM, Remi Forax wrote: > > - Original Message - >> From: "Ceki Gülcü" >> To: "core-libs-dev" >> Sent: Wednesday, April 6, 2022 11:26:39 PM >> Subject: Re: fast way to infer caller > >> Hi Rémi, >> > > Now, i don't think there is a real solution to you issue, worst i will try to > convince you that this is not a real problem :) > You are offering convenience using magic to your user, this has a cost. > > For me this is very similar to the trade off you have by offering to change > the logger configuration at runtime. > This is convenient but it requires a volatile read (even when the logger is > de-activated) which destroy performance in tight loop (you loose hoisting). > I believe that if your users are fine with that, they are also fine with a > call to LoggerFactory.getLogger() being a little slow. > We're only one user, but we use Logback and always assumed getLogger was reasonably expensive, and that the most effort went into optimizing the actual logging itself. So we always store it in a static (preferred) or at least instance field, and if we ever dynamically look it up we assume that's more expensive. To us, the convenience of calling getLogger() over getLogger(MyClass.class) is a nice win, and even 10x the execution speed of calling getLogger doesn't matter at all. If the performance cost was documented we would still use it without hesitation. Just my 2¢, Steven > > >> >> >> On 4/6/2022 5:52 PM, Remi Forax wrote: >>> - Original Message - From: "Ceki Gülcü" To: "core-libs-dev" Sent: Wednesday, April 6, 2022 5:30:51 PM Subject: fast way to infer caller >>> Hello, >>> >>> Hello, >>> As you are probably aware, one of the important primitives used in logging libraries is inferring the caller of a given logging statement. The current common practice is to create a throwable and process its stack trace. This is rather wasteful and rather slow. As an alternative, I have tried using the StackWalker API to infer the caller but was unsatisfied with the performance. MethodHandles.lookup().lookupClass() looks very promising except that there is no way to specify the depth. I am looking for a method to obtain the Nth caller at a cost of around 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater for this use case? >>> >>> We have designed the StackWalker with that in mind >>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html >>> >>> see the discussion on StackWalker.getCallerClass() >>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass()
Re: fast way to infer caller
Hi Jason,
Yes, the code was mentioned in SLF4J PR 271.
The benchmark can be found at
https://github.com/ceki/logback-perf/tree/master/src/main/java/ch/qos/logback/perf/caller
Here is the gist of the StackWalker variant:
public class CallerCompute {
static private StackWalker WALKER = StackWalker.getInstance();
static public String getCallerClass(int depth) {
List frames = WALKER.walk(s -> s.limit(4).toList());
process the returned frames
}
}
The above executes at around 1.8 microseconds per call whereas the code
in LogRecord.getSourceClassName() executes at 5 microseconds per call.
The cost of LogRecord constructor is negligible at around 25 nanoseconds.
Anyway, thank you for the suggestion.
--
Ceki
On 4/7/2022 12:27 AM, Jason Mehrens wrote:
> Ceki,
>
> Looks like the benchmark code is from https://github.com/qos-ch/slf4j/pull/271
>
> Looks like the benchmark code is doing a collection so perhaps that is some
> of the performance hit?
> Have you benchmarked java.util.LogRecord.getSourceClassName() to compare with
> your StackWalker benchmark?
>
> https://github.com/openjdk/jdk/blob/master/src/java.logging/share/classes/java/util/logging/LogRecord.java#L754
>
> Jason
>
>
> From: core-libs-dev on behalf of Ceki
> Gülcü
> Sent: Wednesday, April 6, 2022 4:26 PM
> To: core-libs-dev
> Subject: Re: fast way to infer caller
>
>
> Hi Rémi,
>
> Thank you for your answer.
>
> According to some benchmarks on a i7-8565U Intel CPU (quite average
> CPU), here are the costs of computing the caller class via different
> methods:
>
> using new Throwable().getStackTrace: 11 microseconds per call
> using StackWalker API: 1.8 microseconds per call
> using SecurityManager: 0.9 microseconds per call
>
> While a six fold improvement (StackWalker compared to new Thorowable) is
> nothing to sneeze at, the performance of StackWalker is not as good as
> with a custom SecurityManager.
>
> I have not said so explicitly but the aim here is to allow the user to
> obtain a new logger by calling LoggerFactory.getLogger() with no
> arguments with the returned logger named after the caller class.
>
> Spending 1 or 2 microseconds for this call is OK if the logger is a
> static field. However, if the logger is an instance field, then spending
> 2 microseconds per host object instance just to obtain a logger might
> have a noticeable impact on performance. It follows that the performance
> of LoggerFactory.getLogger() must be exceptionally good, assuming we
> wish to avoid having the user accidentally shooting herself on the foot,
> ergo the 100 nanosecond performance per call requirement.
>
> Noting that invoking MethodHandles.lookup().lookupClass() seems very
> fast (about 2 nanoseconds), I would be very interested if new
> lookup(int index) method were added to java.lang.invoke.MethodHandles
> class, with index designating the desired index on the call stack.
>
> Does the above make sense? How difficult would it be to add such a method?
>
> --
> Ceki Gülcü
>
>
> On 4/6/2022 5:52 PM, Remi Forax wrote:
>> - Original Message -
>>> From: "Ceki Gülcü"
>>> To: "core-libs-dev"
>>> Sent: Wednesday, April 6, 2022 5:30:51 PM
>>> Subject: fast way to infer caller
>>
>>> Hello,
>>
>> Hello,
>>
>>>
>>> As you are probably aware, one of the important primitives used in
>>> logging libraries is inferring the caller of a given logging statement.
>>> The current common practice is to create a throwable and process its
>>> stack trace. This is rather wasteful and rather slow. As an alternative,
>>> I have tried using the StackWalker API to infer the caller but was
>>> unsatisfied with the performance.
>>>
>>> MethodHandles.lookup().lookupClass() looks very promising except that
>>> there is no way to specify the depth.
>>>
>>> I am looking for a method to obtain the Nth caller at a cost of around
>>> 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater
>>> for this use case?
>>
>> We have designed the StackWalker with that in mind
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html
>>
>> see the discussion on StackWalker.getCallerClass()
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass()
>>
>
Re: fast way to infer caller
- Original Message - > From: "Ceki Gülcü" > To: "core-libs-dev" > Sent: Wednesday, April 6, 2022 11:26:39 PM > Subject: Re: fast way to infer caller > Hi Rémi, > > Thank you for your answer. > > According to some benchmarks on a i7-8565U Intel CPU (quite average > CPU), here are the costs of computing the caller class via different > methods: > > using new Throwable().getStackTrace: 11 microseconds per call > using StackWalker API: 1.8 microseconds per call > using SecurityManager: 0.9 microseconds per call > > While a six fold improvement (StackWalker compared to new Throwable) is > nothing to sneeze at, the performance of StackWalker is not as good as > with a custom SecurityManager. > > I have not said so explicitly but the aim here is to allow the user to > obtain a new logger by calling LoggerFactory.getLogger() with no > arguments with the returned logger named after the caller class. > > Spending 1 or 2 microseconds for this call is OK if the logger is a > static field. However, if the logger is an instance field, then spending > 2 microseconds per host object instance just to obtain a logger might > have a noticeable impact on performance. It follows that the performance > of LoggerFactory.getLogger() must be exceptionally good, assuming we > wish to avoid having the user accidentally shooting herself on the foot, > ergo the 100 nanosecond performance per call requirement. > > Noting that invoking MethodHandles.lookup().lookupClass() seems very > fast (about 2 nanoseconds), I would be very interested if new > lookup(int index) method were added to java.lang.invoke.MethodHandles > class, with index designating the desired index on the call stack. > > Does the above make sense? How difficult would it be to add such a method ? I think that for MethodHandles.lookup() and all other caller sensitive methods, the VM uses a trick, it inlines the call into the caller method, once this is done, the caller class is know statically and can be replaced by a constant. In order to use the same trick, you need a way to force the inlining through getLogger(), the is something the JDK can do but that is not available to user code. And you do not want lookup(-1) because getLogger() can be called by reflection, java.lang.invoke or a lambda, in all these cases you have "hidden" stack frames in between the user code code and LoggerFactory.getLogger(), you need to skip those stack frames, this is what the StackWalker does. Now, i don't think there is a real solution to you issue, worst i will try to convince you that this is not a real problem :) You are offering convenience using magic to your user, this has a cost. For me this is very similar to the trade off you have by offering to change the logger configuration at runtime. This is convenient but it requires a volatile read (even when the logger is de-activated) which destroy performance in tight loop (you loose hoisting). I believe that if your users are fine with that, they are also fine with a call to LoggerFactory.getLogger() being a little slow. > > -- > Ceki Gülcü Rémi > > > On 4/6/2022 5:52 PM, Remi Forax wrote: >> - Original Message - >>> From: "Ceki Gülcü" >>> To: "core-libs-dev" >>> Sent: Wednesday, April 6, 2022 5:30:51 PM >>> Subject: fast way to infer caller >> >>> Hello, >> >> Hello, >> >>> >>> As you are probably aware, one of the important primitives used in >>> logging libraries is inferring the caller of a given logging statement. >>> The current common practice is to create a throwable and process its >>> stack trace. This is rather wasteful and rather slow. As an alternative, >>> I have tried using the StackWalker API to infer the caller but was >>> unsatisfied with the performance. >>> >>> MethodHandles.lookup().lookupClass() looks very promising except that >>> there is no way to specify the depth. >>> >>> I am looking for a method to obtain the Nth caller at a cost of around >>> 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater >>> for this use case? >> >> We have designed the StackWalker with that in mind >> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html >> >> see the discussion on StackWalker.getCallerClass() >> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass()
Re: RFR: 8282221: x86 intrinsics for divideUnsigned and remainderUnsigned methods in java.lang.Integer and java.lang.Long [v10]
> Optimizes the divideUnsigned() and remainderUnsigned() methods in > java.lang.Integer and java.lang.Long classes using x86 intrinsics. This > change shows 3x improvement for Integer methods and upto 25% improvement for > Long. This change also implements the DivMod optimization which fuses > division and modulus operations if needed. The DivMod optimization shows 3x > improvement for Integer and ~65% improvement for Long. Srinivas Vamsi Parasa has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits: - use appropriate style changes - Merge branch 'master' of https://git.openjdk.java.net/jdk into udivmod - Merge branch 'openjdk:master' into udivmod - add error msg for jtreg test - update jtreg test to run on x86_64 - add bmi1 support check and jtreg tests - Merge branch 'master' of https://git.openjdk.java.net/jdk into udivmod - fix 32bit build issues - Fix line at end of file - Move intrinsic code to macro assembly routines; remove unused transformations for div and mod nodes - ... and 5 more: https://git.openjdk.java.net/jdk/compare/4451257b...9949047c - Changes: https://git.openjdk.java.net/jdk/pull/7572/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7572&range=09 Stats: 1011 lines in 20 files changed: 1009 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7572/head:pull/7572 PR: https://git.openjdk.java.net/jdk/pull/7572
Re: fast way to infer caller
Ceki, Looks like the benchmark code is from https://github.com/qos-ch/slf4j/pull/271 Looks like the benchmark code is doing a collection so perhaps that is some of the performance hit? Have you benchmarked java.util.LogRecord.getSourceClassName() to compare with your StackWalker benchmark? https://github.com/openjdk/jdk/blob/master/src/java.logging/share/classes/java/util/logging/LogRecord.java#L754 Jason From: core-libs-dev on behalf of Ceki Gülcü Sent: Wednesday, April 6, 2022 4:26 PM To: core-libs-dev Subject: Re: fast way to infer caller Hi Rémi, Thank you for your answer. According to some benchmarks on a i7-8565U Intel CPU (quite average CPU), here are the costs of computing the caller class via different methods: using new Throwable().getStackTrace: 11 microseconds per call using StackWalker API: 1.8 microseconds per call using SecurityManager: 0.9 microseconds per call While a six fold improvement (StackWalker compared to new Thorowable) is nothing to sneeze at, the performance of StackWalker is not as good as with a custom SecurityManager. I have not said so explicitly but the aim here is to allow the user to obtain a new logger by calling LoggerFactory.getLogger() with no arguments with the returned logger named after the caller class. Spending 1 or 2 microseconds for this call is OK if the logger is a static field. However, if the logger is an instance field, then spending 2 microseconds per host object instance just to obtain a logger might have a noticeable impact on performance. It follows that the performance of LoggerFactory.getLogger() must be exceptionally good, assuming we wish to avoid having the user accidentally shooting herself on the foot, ergo the 100 nanosecond performance per call requirement. Noting that invoking MethodHandles.lookup().lookupClass() seems very fast (about 2 nanoseconds), I would be very interested if new lookup(int index) method were added to java.lang.invoke.MethodHandles class, with index designating the desired index on the call stack. Does the above make sense? How difficult would it be to add such a method? -- Ceki Gülcü On 4/6/2022 5:52 PM, Remi Forax wrote: > - Original Message - >> From: "Ceki Gülcü" >> To: "core-libs-dev" >> Sent: Wednesday, April 6, 2022 5:30:51 PM >> Subject: fast way to infer caller > >> Hello, > > Hello, > >> >> As you are probably aware, one of the important primitives used in >> logging libraries is inferring the caller of a given logging statement. >> The current common practice is to create a throwable and process its >> stack trace. This is rather wasteful and rather slow. As an alternative, >> I have tried using the StackWalker API to infer the caller but was >> unsatisfied with the performance. >> >> MethodHandles.lookup().lookupClass() looks very promising except that >> there is no way to specify the depth. >> >> I am looking for a method to obtain the Nth caller at a cost of around >> 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater >> for this use case? > > We have designed the StackWalker with that in mind > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html > > see the discussion on StackWalker.getCallerClass() > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass() >
Re: RFR: 8283892: Compress and expand bits [v2]
On Wed, 6 Apr 2022 17:43:34 GMT, John R Rose wrote: >> Paul Sandoz has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Doc and test updates. > > test/jdk/java/lang/AbstractCompressExpandTest.java line 104: > >> 102: abstract long expectedExpand(long i, long mask); >> 103: >> 104: static int SIZE = 1024; > > It feels like `SIZE` should be a constructor parameter, to make it easier to > run ad hoc stress tests. That's tricky because the test is TestNG based, launched via jtreg, it would need to be a system property. - PR: https://git.openjdk.java.net/jdk/pull/8115
Re: RFR: 8283892: Compress and expand bits [v2]
> Add support to compress bits and expand bits of `int` and `long` values, see > Hacker's Delight (2nd edition), section 7.4. > > Compressing or expanding bits of an `int` or `long` value can be composed to > enable general permutations, and the "sheep and goats" operation (SAG) see > Hacker's Delight (2nd edition), section 7.7. SAG can be used to perform a > stable binary radix sort. > > The compress and expand functionality maps efficiently to hardware > instructions, such as `PEXT` and `PDEP` on x86 hardware. Thus the > implementations can be very efficient on supporting hardware. > Intrinsification will occur in a separate PR. > > This [paper](https://arxiv.org/pdf/1706.00990.pdf) investigates the > beneficial performance impact of the `PDEP` instruction, and by extension the > `expand` method, when applied to the implementation of a bit-vector select > operation in succinct data structures (for example `select(r)` returns the > position of the `r`th 1). > > Testing-wise the approach take is three fold: > 1. Tests compared against simple implementations that are easy to read and > verify against the JDK implementations (which later will also be made > intrinsic). To compensate all tests are also run flipping the test methods > and the methods under test. > 2. Tests composed of compress and expand and vice versa. > 3. Tests with known mask patterns, whose expected values are easily derived > from the inputs. Paul Sandoz has updated the pull request incrementally with one additional commit since the last revision: Doc and test updates. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8115/files - new: https://git.openjdk.java.net/jdk/pull/8115/files/9516ecca..8c536de6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8115&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8115&range=00-01 Stats: 33 lines in 3 files changed: 17 ins; 2 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/8115.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8115/head:pull/8115 PR: https://git.openjdk.java.net/jdk/pull/8115
Re: fast way to infer caller
Hi Rémi, Thank you for your answer. According to some benchmarks on a i7-8565U Intel CPU (quite average CPU), here are the costs of computing the caller class via different methods: using new Throwable().getStackTrace: 11 microseconds per call using StackWalker API: 1.8 microseconds per call using SecurityManager: 0.9 microseconds per call While a six fold improvement (StackWalker compared to new Thorowable) is nothing to sneeze at, the performance of StackWalker is not as good as with a custom SecurityManager. I have not said so explicitly but the aim here is to allow the user to obtain a new logger by calling LoggerFactory.getLogger() with no arguments with the returned logger named after the caller class. Spending 1 or 2 microseconds for this call is OK if the logger is a static field. However, if the logger is an instance field, then spending 2 microseconds per host object instance just to obtain a logger might have a noticeable impact on performance. It follows that the performance of LoggerFactory.getLogger() must be exceptionally good, assuming we wish to avoid having the user accidentally shooting herself on the foot, ergo the 100 nanosecond performance per call requirement. Noting that invoking MethodHandles.lookup().lookupClass() seems very fast (about 2 nanoseconds), I would be very interested if new lookup(int index) method were added to java.lang.invoke.MethodHandles class, with index designating the desired index on the call stack. Does the above make sense? How difficult would it be to add such a method? -- Ceki Gülcü On 4/6/2022 5:52 PM, Remi Forax wrote: > - Original Message - >> From: "Ceki Gülcü" >> To: "core-libs-dev" >> Sent: Wednesday, April 6, 2022 5:30:51 PM >> Subject: fast way to infer caller > >> Hello, > > Hello, > >> >> As you are probably aware, one of the important primitives used in >> logging libraries is inferring the caller of a given logging statement. >> The current common practice is to create a throwable and process its >> stack trace. This is rather wasteful and rather slow. As an alternative, >> I have tried using the StackWalker API to infer the caller but was >> unsatisfied with the performance. >> >> MethodHandles.lookup().lookupClass() looks very promising except that >> there is no way to specify the depth. >> >> I am looking for a method to obtain the Nth caller at a cost of around >> 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater >> for this use case? > > We have designed the StackWalker with that in mind > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html > > see the discussion on StackWalker.getCallerClass() > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass() >
Re: RFR: 8284444: Sting typo [v3]
On Wed, 6 Apr 2022 16:47:17 GMT, Daniel Jeliński wrote: >> This patch adds missing `r` in `string`s > > Daniel Jeliński has updated the pull request incrementally with two > additional commits since the last revision: > > - revert xalan changes > - revert icu changes JFR changes look fine, thank you. - Marked as reviewed by mgronlun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8283698: Refactor Locale constructors used in src/test
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote: > This is a follow-on task after deprecating the Locale constructors > (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are > simple replacements to Locale constructors with `Locale.of()` or Locale > constants, such as `Locale.US`. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: 8283892: Compress and expand bits
On Tue, 5 Apr 2022 22:05:19 GMT, Paul Sandoz wrote:
> Add support to compress bits and expand bits of `int` and `long` values, see
> Hacker's Delight (2nd edition), section 7.4.
>
> Compressing or expanding bits of an `int` or `long` value can be composed to
> enable general permutations, and the "sheep and goats" operation (SAG) see
> Hacker's Delight (2nd edition), section 7.7. SAG can be used to perform a
> stable binary radix sort.
>
> The compress and expand functionality maps efficiently to hardware
> instructions, such as `PEXT` and `PDEP` on x86 hardware. Thus the
> implementations can be very efficient on supporting hardware.
> Intrinsification will occur in a separate PR.
>
> This [paper](https://arxiv.org/pdf/1706.00990.pdf) investigates the
> beneficial performance impact of the `PDEP` instruction, and by extension the
> `expand` method, when applied to the implementation of a bit-vector select
> operation in succinct data structures (for example `select(r)` returns the
> position of the `r`th 1).
>
> Testing-wise the approach take is three fold:
> 1. Tests compared against simple implementations that are easy to read and
> verify against the JDK implementations (which later will also be made
> intrinsic). To compensate all tests are also run flipping the test methods
> and the methods under test.
> 2. Tests composed of compress and expand and vice versa.
> 3. Tests with known mask patterns, whose expected values are easily derived
> from the inputs.
Changes requested by jrose (Reviewer).
src/java.base/share/classes/java/lang/Integer.java line 1781:
> 1779: * All the upper remaining bits of the compressed value are set
> 1780: * to zero.
> 1781: *
Following Alan's comment, I suggest the following examples:
compress(0x9ABCDEF, 0x0F0F0FF) == 0xACEF
compress(x, 1<>n & 1)
compress(x, -1<>> n
compress(m, m) == (m==-1||m==0)? m : (1<>> n) & 1 == /*the bit of x corresponding to the nth set bit
in m*/
…In two places. Similarly, examples for expand:
expand(0x9ABCDEF, 0x0F0F0FF) == 0xC0D0EF
expand(x, 1< 102: abstract long expectedExpand(long i, long mask);
> 103:
> 104: static int SIZE = 1024;
It feels like `SIZE` should be a constructor parameter, to make it easier to
run ad hoc stress tests.
test/jdk/java/lang/AbstractCompressExpandTest.java line 145:
> 143:
> 144: int i = 0;
> 145: for (int len = 0; len < 32; len++) {
Lengths should be `len = 1; len <= 32; len++`, generating fewer redundant
zero-length masks and a full-length -1 mask. The pos should be `pos = 0; pos
<= 32 - len; pos++`, generating fully left-justified masks of the form
`-1
Re: RFR: 8284444: Sting typo [v3]
On Wed, 6 Apr 2022 16:47:17 GMT, Daniel Jeliński wrote: >> This patch adds missing `r` in `string`s > > Daniel Jeliński has updated the pull request incrementally with two > additional commits since the last revision: > > - revert xalan changes > - revert icu changes The changes look fine to me now. Yet there are no changes in the client area any more. So you'll have to get additional approvals. - Marked as reviewed by aivanov (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo [v3]
On Wed, 6 Apr 2022 14:12:49 GMT, Daniel Jeliński wrote:
>> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources.properties
>> line 63:
>>
>>> 61: message.creating-association-with-null-extension=Creating association
>>> with null extension.
>>> 62: message.wrong-tool-version=Detected [{0}] version {1} but version {2}
>>> is required.
>>> 63: message.version-string-too-many-components=Version string may have up
>>> to 3 components - major.minor.build .
>>
>> I wonder whether the space before the period is required at the end of the
>> sentence. Perhaps, it's to separate a property name from the end of the
>> sentence.
>
> right; without the space the period would appear to be part of the version
> pattern.
Yes, I believe this is why it was done.
-
PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo [v3]
On Wed, 6 Apr 2022 16:47:17 GMT, Daniel Jeliński wrote: >> This patch adds missing `r` in `string`s > > Daniel Jeliński has updated the pull request incrementally with two > additional commits since the last revision: > > - revert xalan changes > - revert icu changes The `jpackage` part of the fix looks good. Btw, there are translated files for 3 other languages (German, Japanese, Simplified Chinese), so I double-checked the translation for each, and they already say "string" (I guess whoever translated it just assumed it was meant to be "string" when they took the English phrase). - Marked as reviewed by kcr (Author). PR: https://git.openjdk.java.net/jdk/pull/8125
RFR: 8283698: Refactor Locale constructors used in src/test
This is a follow-on task after deprecating the Locale constructors (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are simple replacements to Locale constructors with `Locale.of()` or Locale constants, such as `Locale.US`. - Commit messages: - 8283698: Refactor Locale constructors used in src/test Changes: https://git.openjdk.java.net/jdk/pull/8130/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8130&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283698 Stats: 750 lines in 182 files changed: 3 ins; 3 del; 744 mod Patch: https://git.openjdk.java.net/jdk/pull/8130.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8130/head:pull/8130 PR: https://git.openjdk.java.net/jdk/pull/8130
Re: RFR: 8283892: Compress and expand bits
On Wed, 6 Apr 2022 14:58:37 GMT, Alan Bateman wrote:
>> Add support to compress bits and expand bits of `int` and `long` values, see
>> Hacker's Delight (2nd edition), section 7.4.
>>
>> Compressing or expanding bits of an `int` or `long` value can be composed to
>> enable general permutations, and the "sheep and goats" operation (SAG) see
>> Hacker's Delight (2nd edition), section 7.7. SAG can be used to perform a
>> stable binary radix sort.
>>
>> The compress and expand functionality maps efficiently to hardware
>> instructions, such as `PEXT` and `PDEP` on x86 hardware. Thus the
>> implementations can be very efficient on supporting hardware.
>> Intrinsification will occur in a separate PR.
>>
>> This [paper](https://arxiv.org/pdf/1706.00990.pdf) investigates the
>> beneficial performance impact of the `PDEP` instruction, and by extension
>> the `expand` method, when applied to the implementation of a bit-vector
>> select operation in succinct data structures (for example `select(r)`
>> returns the position of the `r`th 1).
>>
>> Testing-wise the approach take is three fold:
>> 1. Tests compared against simple implementations that are easy to read and
>> verify against the JDK implementations (which later will also be made
>> intrinsic). To compensate all tests are also run flipping the test methods
>> and the methods under test.
>> 2. Tests composed of compress and expand and vice versa.
>> 3. Tests with known mask patterns, whose expected values are easily derived
>> from the inputs.
>
> src/java.base/share/classes/java/lang/Integer.java line 1775:
>
>> 1773: * the specified bit mask.
>> 1774: *
>> 1775: * For each one-bit value of the mask, {@code mb} say, from least
>
> A minor comments is that "For each one-bit value of the mask mb " might
> be a bit better, otherwise I think these methods and their javadoc looks
> good. If it comes up then these methods could include an example in the
> javadoc as they aren't hard once you see an example.
I can change to "For each one-bit value {@code mb} of the mask ..."
-
PR: https://git.openjdk.java.net/jdk/pull/8115
Re: RFR: 8282221: x86 intrinsics for divideUnsigned and remainderUnsigned methods in java.lang.Integer and java.lang.Long [v8]
On Wed, 6 Apr 2022 00:46:01 GMT, Vladimir Kozlov wrote: > I have few comments. Thank you Vladimir (@vnkozlov) for suggesting the changes! Will incorporate the suggestions and push an update in few hours. - PR: https://git.openjdk.java.net/jdk/pull/7572
Re: RFR: 8282664: Unroll by hand StringUTF16 and StringLatin1 polynomial hash loops [v7]
On Tue, 5 Apr 2022 15:40:29 GMT, Ludovic Henry wrote: >> Despite the hash value being cached for Strings, computing the hash still >> represents a significant CPU usage for applications handling lots of text. >> >> Even though it would be generally better to do it through an enhancement to >> the autovectorizer, the complexity of doing it by hand is trivial and the >> gain is sizable (2x speedup) even without the Vector API. The algorithm has >> been proposed by Richard Startin and Paul Sandoz [1]. >> >> Speedup are as follows on a `Intel(R) Xeon(R) E-2276G CPU @ 3.80GHz` >> >> >> Benchmark(size) Mode Cnt Score >>Error Units >> StringHashCode.Algorithm.scalarLatin1 0 avgt 25 2.111 >> ± 0.210 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 3.500 >> ± 0.127 ns/op >> StringHashCode.Algorithm.scalarLatin110 avgt 25 7.001 >> ± 0.099 ns/op >> StringHashCode.Algorithm.scalarLatin1 100 avgt 2561.285 >> ± 0.444 ns/op >> StringHashCode.Algorithm.scalarLatin1 1000 avgt 25 628.995 >> ± 0.846 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 6307.990 >> ± 4.071 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 0 avgt 25 2.358 >> ± 0.092 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3.631 >> ± 0.159 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 10 avgt 25 7.049 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 100 avgt 2533.626 >> ± 1.218 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled161000 avgt 25 317.811 >> ± 1.225 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3212.333 >> ± 14.621 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled80 avgt 25 2.356 >> ± 0.097 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3.630 >> ± 0.158 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 10 avgt 25 8.724 >> ± 0.065 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 100 avgt 2532.402 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 1000 avgt 25 321.949 >> ± 0.251 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3202.083 >> ± 1.667 ns/op >> StringHashCode.Algorithm.scalarUTF16 0 avgt 25 2.135 >> ± 0.191 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 5.202 >> ± 0.362 ns/op >> StringHashCode.Algorithm.scalarUTF16 10 avgt 2511.105 >> ± 0.112 ns/op >> StringHashCode.Algorithm.scalarUTF16100 avgt 2575.974 >> ± 0.702 ns/op >> StringHashCode.Algorithm.scalarUTF16 1000 avgt 25 716.429 >> ± 3.290 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 7095.459 >> ± 43.847 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled160 avgt 25 2.381 >> ± 0.038 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 5.268 >> ± 0.422 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 10 avgt 2511.248 >> ± 0.178 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 100 avgt 2552.966 >> ± 0.089 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 1000 avgt 25 450.912 >> ± 1.834 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 4403.988 >> ± 2.927 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 0 avgt 25 2.401 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 5.091 >> ± 0.396 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled810 avgt 2512.801 >> ± 0.189 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 100 avgt 2552.068 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1000 avgt 25 453.270 >> ± 0.340 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 4433.112 >> ± 2.699 ns/op >> >> >> At Datadog, we handle a great amount of text (through logs management for >> example), and hashing String represents a large part of our CPU usage. It's >> very unlikely that we are the only one as String.hashCode is such a core >> feature of the JVM-based languages with its use in HashMap for example. >> Having even only a 2x speedup would allow us to save thousands of CPU cores >> per month and improve correspondingly the energy/carbon impact. >> >> [1] >> https://static.rainfocus.com/oracle/oow18/sess/1525822677955001tLqU/PF/codeone18-vector-API-DEV5081_1540354883936001Q3Sv.pdf > > Ludovic Henry has updated the pull request incrementally with two additional > commits since
Re: RFR: 8282664: Unroll by hand StringUTF16 and StringLatin1 polynomial hash loops [v8]
> Despite the hash value being cached for Strings, computing the hash still > represents a significant CPU usage for applications handling lots of text. > > Even though it would be generally better to do it through an enhancement to > the autovectorizer, the complexity of doing it by hand is trivial and the > gain is sizable (2x speedup) even without the Vector API. The algorithm has > been proposed by Richard Startin and Paul Sandoz [1]. > > Speedup are as follows on a `Intel(R) Xeon(R) E-2276G CPU @ 3.80GHz` > > > Benchmark(size) Mode Cnt Score > Error Units > StringHashCode.Algorithm.scalarLatin1 0 avgt 25 2.111 > ± 0.210 ns/op > StringHashCode.Algorithm.scalarLatin1 1 avgt 25 3.500 > ± 0.127 ns/op > StringHashCode.Algorithm.scalarLatin110 avgt 25 7.001 > ± 0.099 ns/op > StringHashCode.Algorithm.scalarLatin1 100 avgt 2561.285 > ± 0.444 ns/op > StringHashCode.Algorithm.scalarLatin1 1000 avgt 25 628.995 > ± 0.846 ns/op > StringHashCode.Algorithm.scalarLatin1 1 avgt 25 6307.990 > ± 4.071 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled16 0 avgt 25 2.358 > ± 0.092 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3.631 > ± 0.159 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled16 10 avgt 25 7.049 > ± 0.019 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled16 100 avgt 2533.626 > ± 1.218 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled161000 avgt 25 317.811 > ± 1.225 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3212.333 > ± 14.621 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled80 avgt 25 2.356 > ± 0.097 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3.630 > ± 0.158 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled8 10 avgt 25 8.724 > ± 0.065 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled8 100 avgt 2532.402 > ± 0.019 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled8 1000 avgt 25 321.949 > ± 0.251 ns/op > StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3202.083 > ± 1.667 ns/op > StringHashCode.Algorithm.scalarUTF16 0 avgt 25 2.135 > ± 0.191 ns/op > StringHashCode.Algorithm.scalarUTF16 1 avgt 25 5.202 > ± 0.362 ns/op > StringHashCode.Algorithm.scalarUTF16 10 avgt 2511.105 > ± 0.112 ns/op > StringHashCode.Algorithm.scalarUTF16100 avgt 2575.974 > ± 0.702 ns/op > StringHashCode.Algorithm.scalarUTF16 1000 avgt 25 716.429 > ± 3.290 ns/op > StringHashCode.Algorithm.scalarUTF16 1 avgt 25 7095.459 > ± 43.847 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled160 avgt 25 2.381 > ± 0.038 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 5.268 > ± 0.422 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled16 10 avgt 2511.248 > ± 0.178 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled16 100 avgt 2552.966 > ± 0.089 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled16 1000 avgt 25 450.912 > ± 1.834 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 4403.988 > ± 2.927 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled8 0 avgt 25 2.401 > ± 0.032 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 5.091 > ± 0.396 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled810 avgt 2512.801 > ± 0.189 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled8 100 avgt 2552.068 > ± 0.032 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled8 1000 avgt 25 453.270 > ± 0.340 ns/op > StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 4433.112 > ± 2.699 ns/op > > > At Datadog, we handle a great amount of text (through logs management for > example), and hashing String represents a large part of our CPU usage. It's > very unlikely that we are the only one as String.hashCode is such a core > feature of the JVM-based languages with its use in HashMap for example. > Having even only a 2x speedup would allow us to save thousands of CPU cores > per month and improve correspondingly the energy/carbon impact. > > [1] > https://static.rainfocus.com/oracle/oow18/sess/1525822677955001tLqU/PF/codeone18-vector-API-DEV5081_1540354883936001Q3Sv.pdf Ludovic Henry has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last rev
Re: RFR: 8284444: Sting typo [v2]
On Wed, 6 Apr 2022 16:48:38 GMT, Daniel Jeliński wrote: >> src/java.base/share/classes/jdk/internal/icu/text/StringPrep.java line 63: >> >>> 61: * http://www.ietf.org/rfc/rfc3454.txt";>RFC 3454. >>> 62: * StringPrep prepares Unicode strings for use in network protocols. >>> 63: * Profiles of StringPrep are set of rules and data according to which >>> the >> >> These also come from the upstream ICU4J project and should be corrected >> there. >> https://github.com/unicode-org/icu/blob/4833cc89b2fae2e8863b46bf1dc785964847e882/icu4j/main/classes/core/src/com/ibm/icu/text/StringPrep.java#L27 > > Thanks, reverted. Also reverted Xalan changes. Good. Thanks for reverting it. - PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo [v2]
On Wed, 6 Apr 2022 15:49:06 GMT, Naoto Sato wrote: >> Daniel Jeliński has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Revert liblcms changes > > src/java.base/share/classes/jdk/internal/icu/text/StringPrep.java line 63: > >> 61: * http://www.ietf.org/rfc/rfc3454.txt";>RFC 3454. >> 62: * StringPrep prepares Unicode strings for use in network protocols. >> 63: * Profiles of StringPrep are set of rules and data according to which >> the > > These also come from the upstream ICU4J project and should be corrected there. > https://github.com/unicode-org/icu/blob/4833cc89b2fae2e8863b46bf1dc785964847e882/icu4j/main/classes/core/src/com/ibm/icu/text/StringPrep.java#L27 Thanks, reverted. Also reverted Xalan changes. - PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo [v3]
> This patch adds missing `r` in `string`s Daniel Jeliński has updated the pull request incrementally with two additional commits since the last revision: - revert xalan changes - revert icu changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/8125/files - new: https://git.openjdk.java.net/jdk/pull/8125/files/68ce6ebd..1ec0314e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8125&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8125&range=01-02 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8125.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8125/head:pull/8125 PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8279876: Clean up: isAssignableFrom usages in xpath and jdk internal classes [v2]
On Wed, 6 Apr 2022 15:54:30 GMT, Naoto Sato wrote: > Looks good to me. This would have been a premiere example for switch pattern > match, but hey. Yeah, totally understand the urge to use new features. - PR: https://git.openjdk.java.net/jdk/pull/8116
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v3]
On Tue, 29 Mar 2022 12:43:25 GMT, Volker Simonis wrote: >> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)` >> to highlight that it might write more bytes than the returned number of >> inflated bytes into the buffer `b`. >> >> The superclass `java.io.InputStream` specifies that `read(byte[] b, int off, >> int len)` will leave the content beyond the last read byte in the read >> buffer `b` unaffected. However, the overridden `read` method in >> `InflaterInputStream` passes the read buffer `b` to >> `Inflater::inflate(byte[] b, int off, int len)` which doesn't provide this >> guarantee. Depending on implementation details, `Inflater::inflate` might >> write more than the returned number of inflated bytes into the buffer `b`. >> >> ### TL;DR >> >> `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater >> functionality. `Inflater::inflate(byte[] output, int off, int len)` >> currently calls zlib's native `inflate(..)` function and passes the address >> of `output[off]` and `len` to it via JNI. >> >> The specification of zlib's `inflate(..)` function (i.e. the [API >> documentation in the original zlib >> implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) >> doesn't give any guarantees with regard to usage of the output buffer. It >> only states that upon completion the function will return the number of >> bytes that have been written (i.e. "inflated") into the output buffer. >> >> The original zlib implementation only wrote as many bytes into the output >> buffer as it inflated. However, this is not a hard requirement and newer, >> more performant implementations of the zlib library like >> [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) >> or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes >> of the output buffer than they actually inflate as a scratch buffer. See >> https://github.com/simonis/zlib-chromium for a more detailed description of >> their approach and its performance benefit. >> >> These new zlib versions can still be used transparently from Java (e.g. by >> putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because >> they still fully comply to specification of `Inflater::inflate(..)`. >> However, we might run into problems when using the `Inflater` functionality >> from the `InflaterInputStream` class. `InflaterInputStream` is derived from >> from `InputStream` and as such, its `read(byte[] b, int off, int len)` >> method is quite constrained. It specifically specifies that if *k* bytes >> have been read, then "these bytes will be stored in elements `b[off]` >> through `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through >> `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] b, int >> off, int len)` (which is constrained by `InputStream::read(..)`'s >> specification) calls `Inflater::inflate(byte[] b, int off, int len)` and >> directly passes its output buffer down to the native zlib `inflate(..)` >> method which is free to change the bytes beyond `b[off+` *k*`]` (where *k* is the number of inflated bytes). >> >> From a practical point of view, I don't see this as a big problem, because >> callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never >> know how many bytes will be written into the output buffer `b` (and in fact >> its content can always be completely overwritten). It therefore makes no >> sense to depend on any data there being untouched after the call. Also, >> having used zlib-cloudflare productively for about two years, we haven't >> seen real-world issues because of this behavior yet. However, from a >> specification point of view it is easy to artificially construct a program >> which violates `InflaterInputStream::read(..)`'s postcondition if using one >> of the alterantive zlib implementations. A recently integrated JTreg test >> (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails >> with zlib-chromium but can fixed easily to run with alternative >> implementations as well (see >> [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)). > > Volker Simonis has updated the pull request incrementally with one additional > commit since the last revision: > > Extended API-doc based on reviewer feedback Hi, I've pushed a new version which: - improves the wording as suggested - adds the API amendments from `InflaterInputStream::read()` to `GZIPInputStream::read()`/`ZipInputStream::read()` as well be cause they both inherit from `InflaterInputStream`. Unfortunately it's not possible to simply inherit the API doc because of different parameter naming. - added an `@implNote` to `ZipFile::getInputStream()` to make it clear that this implementation really returns an `InflaterInputStream`. Please let me know what you think? Volker - PR: https://git.openjdk.jav
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v4]
> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)` to > highlight that it might write more bytes than the returned number of > inflated bytes into the buffer `b`. > > The superclass `java.io.InputStream` specifies that `read(byte[] b, int off, > int len)` will leave the content beyond the last read byte in the read buffer > `b` unaffected. However, the overridden `read` method in > `InflaterInputStream` passes the read buffer `b` to `Inflater::inflate(byte[] > b, int off, int len)` which doesn't provide this guarantee. Depending on > implementation details, `Inflater::inflate` might write more than the > returned number of inflated bytes into the buffer `b`. > > ### TL;DR > > `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater > functionality. `Inflater::inflate(byte[] output, int off, int len)` currently > calls zlib's native `inflate(..)` function and passes the address of > `output[off]` and `len` to it via JNI. > > The specification of zlib's `inflate(..)` function (i.e. the [API > documentation in the original zlib > implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) > doesn't give any guarantees with regard to usage of the output buffer. It > only states that upon completion the function will return the number of bytes > that have been written (i.e. "inflated") into the output buffer. > > The original zlib implementation only wrote as many bytes into the output > buffer as it inflated. However, this is not a hard requirement and newer, > more performant implementations of the zlib library like > [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) > or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes > of the output buffer than they actually inflate as a scratch buffer. See > https://github.com/simonis/zlib-chromium for a more detailed description of > their approach and its performance benefit. > > These new zlib versions can still be used transparently from Java (e.g. by > putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because > they still fully comply to specification of `Inflater::inflate(..)`. However, > we might run into problems when using the `Inflater` functionality from the > `InflaterInputStream` class. `InflaterInputStream` is derived from from > `InputStream` and as such, its `read(byte[] b, int off, int len)` method is > quite constrained. It specifically specifies that if *k* bytes have been > read, then "these bytes will be stored in elements `b[off]` through > `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through `b[off+len-1]` > **unaffected**". But `InflaterInputStream::read(byte[] b, int off, int len)` > (which is constrained by `InputStream::read(..)`'s specification) calls > `Inflater::inflate(byte[] b, int off, int len)` and directly passes its > output buffer down to the native zlib `inflate(..)` method which is free to > change the bytes beyond `b[off+`* k*`]` (where *k* is the number of inflated bytes). > > From a practical point of view, I don't see this as a big problem, because > callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never > know how many bytes will be written into the output buffer `b` (and in fact > its content can always be completely overwritten). It therefore makes no > sense to depend on any data there being untouched after the call. Also, > having used zlib-cloudflare productively for about two years, we haven't seen > real-world issues because of this behavior yet. However, from a specification > point of view it is easy to artificially construct a program which violates > `InflaterInputStream::read(..)`'s postcondition if using one of the > alterantive zlib implementations. A recently integrated JTreg test > (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails > with zlib-chromium but can fixed easily to run with alternative > implementations as well (see > [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)). Volker Simonis has updated the pull request incrementally with one additional commit since the last revision: Added API-refinement to GZIPInputStream::read()/ZipInputStream::read() and an Implementation note to ZipFile::getInputStream() - Changes: - all: https://git.openjdk.java.net/jdk/pull/7986/files - new: https://git.openjdk.java.net/jdk/pull/7986/files/62a46591..7c671aa5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7986&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7986&range=02-03 Stats: 28 lines in 4 files changed: 19 ins; 2 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/7986.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7986/head:pull/7986 PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8186958: Need method to create pre-sized HashMap [v12]
On Wed, 6 Apr 2022 16:02:31 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > use (double) DEFAULT_LOAD_FACTOR instead of 0.75 There be another question: Should we add `@ForceInline` annotation to the new added functions? as they be static short functions, thus seems maybe benificial to inline them. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8279876: Clean up: isAssignableFrom usages in xpath and jdk internal classes [v2]
On Wed, 6 Apr 2022 01:23:15 GMT, Joe Wang wrote: >> Clean up the usages of isAssignableFrom in a few xpath and jdk/internal >> classes where the checks were really about equality or whether they were the >> exact class types. It was why they worked nonetheless even though some of >> them were backwards. >> >> Test: existing tests passed. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > replace with instanceof Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8116
Re: RFR: 8186958: Need method to create pre-sized HashMap [v10]
On Wed, 6 Apr 2022 15:57:55 GMT, Alan Bateman wrote: > I suspect the core-libs label was added when you created it but you've > expanded it greatly since. Is there a way for making the bot re-calculate the labels? - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v11]
On Wed, 6 Apr 2022 15:55:11 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with two additional > commits since the last revision: > > - drop CalculateHashMapCapacityTestJMH > - refine javadoc for LinkedHashMap#newLinkedHashMap > If you plan to separate the usages into a separate PR @AlanBateman No, my original plan is to make them all in this PR. However if there be reviewer with permission who stronglly opposed to this plan, I can accept making another seperate pr for the changes of usages. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v10]
On Wed, 6 Apr 2022 02:38:17 GMT, Stuart Marks wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> revert changes in jdk.compile
>
> src/java.base/share/classes/java/util/HashMap.java line 2556:
>
>> 2554: */
>> 2555: static int calculateHashMapCapacity(int numMappings) {
>> 2556: return (int) Math.ceil(numMappings / 0.75);
>
> Please use `(double) DEFAULT_LOAD_FACTOR` instead of `0.75`.
@stuart-marks done.
-
PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v10]
On Wed, 6 Apr 2022 15:47:50 GMT, XenoAmess wrote: > I plan to include these changes to issue JDK-8186958, as I don't think it > better to flood about 50 P5 issues and do them seperately in every places. > > That would be a nightmare for reviewers. I didn't ask for 50 PRs, I just asked if you plan to separate the usages into a separate PR. As it stands, most of the areas that you've touched are not notified by this PR. I suspect the core-libs label was added when you created it but you've expanded it greatly since. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v12]
> 8186958: Need method to create pre-sized HashMap XenoAmess has updated the pull request incrementally with one additional commit since the last revision: use (double) DEFAULT_LOAD_FACTOR instead of 0.75 - Changes: - all: https://git.openjdk.java.net/jdk/pull/7928/files - new: https://git.openjdk.java.net/jdk/pull/7928/files/4769b87b..b973ee51 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7928&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7928&range=10-11 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7928.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7928/head:pull/7928 PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8279876: Clean up: isAssignableFrom usages in xpath and jdk internal classes [v2]
On Wed, 6 Apr 2022 01:23:15 GMT, Joe Wang wrote: >> Clean up the usages of isAssignableFrom in a few xpath and jdk/internal >> classes where the checks were really about equality or whether they were the >> exact class types. It was why they worked nonetheless even though some of >> them were backwards. >> >> Test: existing tests passed. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > replace with instanceof Looks good to me. This would have been a premiere example for switch pattern match, but hey. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8116
Re: RFR: 8186958: Need method to create pre-sized HashMap [v10]
On Wed, 6 Apr 2022 00:54:41 GMT, Stuart Marks wrote: >> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> revert changes in jdk.compile > > src/java.base/share/classes/java/util/LinkedHashMap.java line 792: > >> 790: >> 791: /** >> 792: * Creates a new, empty LinkedHashMap suitable for the expected >> number of mappings. > > Adjust wording here to read, > > Creates a new, empty, insertion-ordered LinkedHashMap suitable... > > I've used this wording in the CSR. @stuart-marks done. > I don't think we need this benchmark in this PR. @stuart-marks OK, that benchmark removed. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v10]
On Wed, 6 Apr 2022 14:27:45 GMT, Alan Bateman wrote: > The current patch touches usages all over the JDK. Is that for illustration > purposes or are you planning to include them with the methods? @AlanBateman I plan to include these changes to issue JDK-8186958, as I don't think it better to flood about 50 P5 issues and do them seperately in every places. That would be a nightmare for reviewers. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v11]
> 8186958: Need method to create pre-sized HashMap XenoAmess has updated the pull request incrementally with two additional commits since the last revision: - drop CalculateHashMapCapacityTestJMH - refine javadoc for LinkedHashMap#newLinkedHashMap - Changes: - all: https://git.openjdk.java.net/jdk/pull/7928/files - new: https://git.openjdk.java.net/jdk/pull/7928/files/796975b0..4769b87b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7928&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7928&range=09-10 Stats: 116 lines in 2 files changed: 0 ins; 115 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7928.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7928/head:pull/7928 PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8284444: Sting typo [v2]
On Wed, 6 Apr 2022 14:12:17 GMT, Daniel Jeliński wrote: >> This patch adds missing `r` in `string`s > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Revert liblcms changes src/java.base/share/classes/jdk/internal/icu/text/StringPrep.java line 63: > 61: * http://www.ietf.org/rfc/rfc3454.txt";>RFC 3454. > 62: * StringPrep prepares Unicode strings for use in network protocols. > 63: * Profiles of StringPrep are set of rules and data according to which the These also come from the upstream ICU4J project and should be corrected there. https://github.com/unicode-org/icu/blob/4833cc89b2fae2e8863b46bf1dc785964847e882/icu4j/main/classes/core/src/com/ibm/icu/text/StringPrep.java#L27 - PR: https://git.openjdk.java.net/jdk/pull/8125
Re: fast way to infer caller
- Original Message - > From: "Ceki Gülcü" > To: "core-libs-dev" > Sent: Wednesday, April 6, 2022 5:30:51 PM > Subject: fast way to infer caller > Hello, Hello, > > As you are probably aware, one of the important primitives used in > logging libraries is inferring the caller of a given logging statement. > The current common practice is to create a throwable and process its > stack trace. This is rather wasteful and rather slow. As an alternative, > I have tried using the StackWalker API to infer the caller but was > unsatisfied with the performance. > > MethodHandles.lookup().lookupClass() looks very promising except that > there is no way to specify the depth. > > I am looking for a method to obtain the Nth caller at a cost of around > 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater > for this use case? We have designed the StackWalker with that in mind https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html see the discussion on StackWalker.getCallerClass() https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass() > > -- > Ceki Gülcü > > Sponsoring SLF4J/logback/reload4j at https://github.com/sponsors/qos-ch regards, Rémi
fast way to infer caller
Hello, As you are probably aware, one of the important primitives used in logging libraries is inferring the caller of a given logging statement. The current common practice is to create a throwable and process its stack trace. This is rather wasteful and rather slow. As an alternative, I have tried using the StackWalker API to infer the caller but was unsatisfied with the performance. MethodHandles.lookup().lookupClass() looks very promising except that there is no way to specify the depth. I am looking for a method to obtain the Nth caller at a cost of around 100 to 200 nanoseconds of CPU time. Do you think the JDK could cater for this use case? -- Ceki Gülcü Sponsoring SLF4J/logback/reload4j at https://github.com/sponsors/qos-ch
Re: RFR: 8283892: Compress and expand bits
On Tue, 5 Apr 2022 22:05:19 GMT, Paul Sandoz wrote:
> Add support to compress bits and expand bits of `int` and `long` values, see
> Hacker's Delight (2nd edition), section 7.4.
>
> Compressing or expanding bits of an `int` or `long` value can be composed to
> enable general permutations, and the "sheep and goats" operation (SAG) see
> Hacker's Delight (2nd edition), section 7.7. SAG can be used to perform a
> stable binary radix sort.
>
> The compress and expand functionality maps efficiently to hardware
> instructions, such as `PEXT` and `PDEP` on x86 hardware. Thus the
> implementations can be very efficient on supporting hardware.
> Intrinsification will occur in a separate PR.
>
> This [paper](https://arxiv.org/pdf/1706.00990.pdf) investigates the
> beneficial performance impact of the `PDEP` instruction, and by extension the
> `expand` method, when applied to the implementation of a bit-vector select
> operation in succinct data structures (for example `select(r)` returns the
> position of the `r`th 1).
>
> Testing-wise the approach take is three fold:
> 1. Tests compared against simple implementations that are easy to read and
> verify against the JDK implementations (which later will also be made
> intrinsic). To compensate all tests are also run flipping the test methods
> and the methods under test.
> 2. Tests composed of compress and expand and vice versa.
> 3. Tests with known mask patterns, whose expected values are easily derived
> from the inputs.
src/java.base/share/classes/java/lang/Integer.java line 1775:
> 1773: * the specified bit mask.
> 1774: *
> 1775: * For each one-bit value of the mask, {@code mb} say, from least
A minor comments is that "For each one-bit value of the mask mb " might be
a bit better, otherwise I think these methods and their javadoc looks good. If
it comes up then these methods could include an example in the javadoc as they
aren't hard once you see an example.
-
PR: https://git.openjdk.java.net/jdk/pull/8115
Re: RFR: 8186958: Need method to create pre-sized HashMap [v10]
On Sat, 2 Apr 2022 22:46:19 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > revert changes in jdk.compile The current patch touches usages all over the JDK. Is that for illustration purposes or are you planning to include them with the methods? - PR: https://git.openjdk.java.net/jdk/pull/7928
Integrated: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. This pull request has now been integrated. Changeset: a3851423 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/a385142398eee102ff1a53d848230dc95c4ebd37 Stats: 22 lines in 4 files changed: 14 ins; 0 del; 8 mod 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method Reviewed-by: darcy, shade, coleenp - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8284444: Sting typo [v2]
On Wed, 6 Apr 2022 13:36:05 GMT, Alexey Ivanov wrote:
>> Daniel Jeliński has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Revert liblcms changes
>
> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources.properties
> line 63:
>
>> 61: message.creating-association-with-null-extension=Creating association
>> with null extension.
>> 62: message.wrong-tool-version=Detected [{0}] version {1} but version {2} is
>> required.
>> 63: message.version-string-too-many-components=Version string may have up to
>> 3 components - major.minor.build .
>
> I wonder whether the space before the period is required at the end of the
> sentence. Perhaps, it's to separate a property name from the end of the
> sentence.
right; without the space the period would appear to be part of the version
pattern.
-
PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo [v2]
> This patch adds missing `r` in `string`s Daniel Jeliński has updated the pull request incrementally with one additional commit since the last revision: Revert liblcms changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/8125/files - new: https://git.openjdk.java.net/jdk/pull/8125/files/755c7084..68ce6ebd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8125&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8125&range=00-01 Stats: 12 lines in 1 file changed: 0 ins; 0 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/8125.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8125/head:pull/8125 PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo
On Wed, 6 Apr 2022 13:38:03 GMT, Alexey Ivanov wrote: >> This patch adds missing `r` in `string`s > > src/java.desktop/share/native/liblcms/cmstypes.c line 3668: > >> 3666: // Auxiliary, read an string specified as count + string >> 3667: static >> 3668: cmsBool ReadCountAndString(struct _cms_typehandler_struct* self, >> cmsIOHANDLER* io, cmsMLU* mlu, cmsUInt32Number* SizeOfTag, const char* >> Section) > > As @kevinrushforth mentioned, we usually don't modify the source from > external libraries. You should report and fix the problem upstream. Filed https://github.com/mm2/Little-CMS/pull/313 for this - PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8284444: Sting typo
On Wed, 6 Apr 2022 12:07:30 GMT, Daniel Jeliński wrote:
> This patch adds missing `r` in `string`s
src/java.desktop/share/native/liblcms/cmstypes.c line 3668:
> 3666: // Auxiliary, read an string specified as count + string
> 3667: static
> 3668: cmsBool ReadCountAndString(struct _cms_typehandler_struct* self,
> cmsIOHANDLER* io, cmsMLU* mlu, cmsUInt32Number* SizeOfTag, const char*
> Section)
As @kevinrushforth mentioned, we usually don't modify the source from external
libraries. You should report and fix the problem upstream.
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources.properties
line 63:
> 61: message.creating-association-with-null-extension=Creating association
> with null extension.
> 62: message.wrong-tool-version=Detected [{0}] version {1} but version {2} is
> required.
> 63: message.version-string-too-many-components=Version string may have up to
> 3 components - major.minor.build .
I wonder whether the space before the period is required at the end of the
sentence. Perhaps, it's to separate a property name from the end of the
sentence.
-
PR: https://git.openjdk.java.net/jdk/pull/8125
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. Looks good. - Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8284444: Sting typo
On Wed, 6 Apr 2022 12:07:30 GMT, Daniel Jeliński wrote: > This patch adds missing `r` in `string`s This PR cuts across many areas, so will need multiple reviewers. Regarding the LCMS file, we typically don't make these kind of changes in third-party code, since it will cause our code to diverge from the upstream code, which can lead to merge conflicts down the road. @prrace will need to approve this or else you will need to revert that file. - PR: https://git.openjdk.java.net/jdk/pull/8125
RFR: 8284444: Sting typo
This patch adds missing `r` in `string`s - Commit messages: - Fix sting typo Changes: https://git.openjdk.java.net/jdk/pull/8125/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8125&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-828 Stats: 25 lines in 8 files changed: 0 ins; 0 del; 25 mod Patch: https://git.openjdk.java.net/jdk/pull/8125.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8125/head:pull/8125 PR: https://git.openjdk.java.net/jdk/pull/8125
Integrated: 8284067: jpackage'd launcher reports non-zero exit codes with error prompt
On Thu, 31 Mar 2022 20:08:13 GMT, Alexey Semenyuk wrote: > Add missing `exit(exitCode)` call. > Add relevant unit test. This pull request has now been integrated. Changeset: b9cc3bc1 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/b9cc3bc1bf61572f2813f057eea7326fd0c2bd14 Stats: 26 lines in 4 files changed: 17 ins; 4 del; 5 mod 8284067: jpackage'd launcher reports non-zero exit codes with error prompt Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/8064
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Wed, 6 Apr 2022 09:42:21 GMT, Claes Redestad wrote: > Do I need a second reviewer for the hotspot changes? FWIW, I think hotspot changes are quite simple, so maybe no need for second reviewer? - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. (adding a note to trigger email notification, as that appears lost...) - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Wed, 6 Apr 2022 07:58:33 GMT, Aleksey Shipilev wrote: >> As an alternative to #7667 I took a look at injecting an empty class array >> from the VM. Turns out we already do this for exception types - see >> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 >> - and we can do similarly for the parameter types array. We still need to >> parse the signature for the return type, though. >> >> I've verified by dumping and inspecting heaps that this means we are not >> allocating extra `Class[]` on `Method` reflection. > > Looks fine. I would have thought to put `assert(parameter_count > 0)` near > `mirrors->obj_at_put(index++, mirror);`, but I think it is fine as it is, > since `obj_at_put` does its own bounds checking. Thanks for reviewing! As @shipilev point out `obj_at_put` would assert if we tried to put anything into an empty array, so it seems redundant to add more checks here. Perhaps a comment to point that out since it was brought up here. Do I need a second reviewer for the hotspot changes? - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8240903: Add test to check that jmod hashes are reproducible [v3]
On Fri, 25 Mar 2022 11:54:52 GMT, Alan Bateman wrote: >> Dongbo He has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Get date by 'date +%Y-%m-%dT%H:%M:%S%:z' > > The existing tests for the jmod tool are in test/jdk/tools/jmod. > HashesTest.java might provide inspiration to avoid introducing a shell test. @AlanBateman Could you please take another look? - PR: https://git.openjdk.java.net/jdk/pull/7948
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. Looks fine. I would have thought to put `assert(parameter_count > 0)` near `mirrors->obj_at_put(index++, mirror);`, but I think it is fine as it is, since `obj_at_put` does its own bounds checking. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8283994: Make Xerces DatatypeException stackless [v2]
On Fri, 1 Apr 2022 08:10:48 GMT, Aleksey Shipilev wrote: >> Aleksey Shipilev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Also update LastModified > > Any other reviews? I would like someone else to confirm my investigation that > we don't use the stack traces out of these exceptions too... > Hello @shipilev, do you think this stackless nature of this specific > `DatatypeException` type should be noted in its javadoc, just to avoid any > surprises when someone in future ends up using this exception type as the > "cause" of some other exception? I don't think so. It seems to me the intent for these exceptions is to carry error information without any stack trace info. - PR: https://git.openjdk.java.net/jdk/pull/8036
