Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v4]

2022-08-06 Thread Julian Waters
> Please review a small patch for dumping the failure reason when the MSVCRT 
> libraries or the Java Virtual Machine fails to load on Windows, which can 
> provide invaluable insight when debugging related launcher issues. This also 
> fixes a small bug in the Windows implementation of JLI_ErrorMessageSys where 
> C runtime errors are never written to stderr.

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Back out change to DLL_ERROR4 for separate RFE

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9749/files
  - new: https://git.openjdk.org/jdk/pull/9749/files/3bd53488..990ee4fd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9749=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=9749=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/9749.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9749/head:pull/9749

PR: https://git.openjdk.org/jdk/pull/9749


[BUG] Collections.unmodifiableMap(map).entrySet().iterator().forEachRemaining is missing null check

2022-08-06 Thread Rob Spoor

Hi,

I was working on upgrading another library of mine from Java 8 to 11, 
and I noticed my unit tests started failing. Some debugging traced the 
cause back to java.util.Collections$UnmodifiableMap$UnmodifiableEntrySet 
and its iterator() implementation.


In Java 8, the implementation only implemented hasNext, next and remove. 
It inherited forEachRemaining from java.util.Iterator, which first does 
a null check.


Since Java 11, the forEachRemaining is overridden:

public void forEachRemaining(Consumer> action) {
i.forEachRemaining(entryConsumer(action));
}

The thing is, entryConsumer does not perform any null checks. That's 
done before entryConsumer is called in all occurrences but this one. The 
result is that a non-null action is passed to i.forEachRemaining, which 
causes the iterator to move forward before a NullPointerException is 
thrown. The solution is of course simple: add 
Objects.requireNonNull(action); to the method.


Rob


Re: RFR: 8291641: Optimize StackTraceElement.toString() [v7]

2022-08-06 Thread David Schlosnagle
> I would like to contribute an optimized version of 
> `StackTraceElement#toString()` that uses a single StringBuilder throughout 
> creation to avoid intermediate `String` allocations. 
> `StackTraceElement#toString()` is used in a number of JDK code paths 
> including `Throwable#printStackTrace()`, as well as many JDK consumers may 
> transform `StackTraceElement` `toString()` in logging frameworks capturing 
> throwables and exceptions, and diagnostics performing dumps.
> 
> Given this usage and some observed JFR profiles from production services, I'd 
> like to reduce the intermediate allocations to reduce CPU pressure in these 
> circumstances. I have added a couple benchmarks for a sample 
> `Throwable#printStackTrace()` converted to String via `StringWriter` and 
> individual `StackTraceElement` `toString`. The former shows ~15% improvement, 
> while the latter shows ~40% improvement.
> 
> Before
> 
> Benchmark   Mode  Cnt   Score  Error  
> Units
> StackTraceElementBench.printStackTrace  avgt   15  167147.066 ± 4260.521  
> ns/op
> StackTraceElementBench.toString avgt   15 132.781 ±2.095  
> ns/op
> 
> 
> After
> 
> Benchmark   Mode  Cnt   Score  Error  
> Units
> StackTraceElementBench.printStackTrace  avgt   15  142909.133 ± 2290.720  
> ns/op
> StackTraceElementBench.toString avgt   15  78.939 ±0.469  
> ns/op

David Schlosnagle 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 11 additional commits 
since the last revision:

 - Merge remote-tracking branch 'origin/master' into ds/StackTraceElement
 - Mark StackTraceElement SerialTest for bug 8291641
 - Revert "Mark StackTraceElement SerialTest for bug 8291641"
   
   This reverts commit e7b04faafb026e61829c81c75121e2d3be6644d9.
 - Mark StackTraceElement SerialTest for bug 8291641
 - Inline max Integer.stringSize
 - Estimate length
 - Address comments
 - Precompute StackTraceElement toString length
 - Merge remote-tracking branch 'origin/master' into ds/StackTraceElement
 - Optimize StackTraceElement.toString()
 - ... and 1 more: https://git.openjdk.org/jdk/compare/ba1acce5...c9ae3897

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9665/files
  - new: https://git.openjdk.org/jdk/pull/9665/files/c93ac5af..c9ae3897

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9665=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=9665=05-06

  Stats: 7078 lines in 345 files changed: 4410 ins; 1722 del; 946 mod
  Patch: https://git.openjdk.org/jdk/pull/9665.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9665/head:pull/9665

PR: https://git.openjdk.org/jdk/pull/9665


Re: [jdk19] RFR: 8278274: Update nroff pages in JDK 19 before RC

2022-08-06 Thread Patrick Pfeifer
On Sat, 6 Aug 2022 09:01:39 GMT, Patrick Pfeifer  wrote:

>> src/java.base/share/man/keytool.1 line 456:
>> 
>>> 454: \f[CB]PrivateKeyEntry\f[R] for the signer that already exists in the
>>> 455: keystore.
>>> 456: This option is used to sign the certificate with the signer?s private
>> 
>> Not a problem with this PR as such, but we still have a `?` character in the 
>> output.
>
> Hello @jonathan-gibbons
> 
> Excuse my hijacking / piggy-backing on this conversation!
> 
> When you say
> 
>> Not a problem with this PR as such, but we still have a `?` character in the 
>> output.
> 
> This strongly suggests that there must be an input to those man pages 
> somewhere. :-) Would you mind to point me to it? Are they even included in 
> the git repo?
> 
> Also, I was wondering why these man pages (i.e. the outputs) are obviously 
> not included any more in the OpenJDK builds, e.g. https://jdk.java.net/19/ ? 
> I am not sure when they were removed or if they were ever included in 
> _open_jdk builds at all, but I see them in the Oracle JDK 1.8 Build.
> 
> I find them really nice to read! Are there other output formats, e.g. HTML, 
> that might be are deployed somewhere on the web?

It did not take much effort to answer the last of the above question myself.

> Are there other output formats, e.g. HTML, that might be are deployed 
> somewhere on the web?

Obviously "yes". HTML renderings can be found by following the, e.g. "[Java 
Development Kit Version 18 Tool 
Specifications](https://docs.oracle.com/en/java/javase/18/docs/specs/man/index.html)"
 Link on the "Specifications Overview" page at, e.g. 
"https://docs.oracle.com/en/java/javase/18/docs/specs/;.

Still hunting for the sources.

-

PR: https://git.openjdk.org/jdk19/pull/145


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Thomas Stuefe
On Thu, 4 Aug 2022 17:42:45 GMT, Julian Waters  wrote:

>> Please review a small patch for dumping the failure reason when the MSVCRT 
>> libraries or the Java Virtual Machine fails to load on Windows, which can 
>> provide invaluable insight when debugging related launcher issues. This also 
>> fixes a small bug in the Windows implementation of JLI_ErrorMessageSys where 
>> C runtime errors are never written to stderr.
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Missing spacing between errors

Hi Julian,

> > Hi Julian,
> > I don't get it.
> > AFAICS the existing version displays the failure reason for win32 API just 
> > fine, appending it to the message. Where is the bug? I see that the message 
> > was omitted for CRT errors, so that is okay to fix, but that could have 
> > been fixed with a simple freeit=1 in the CRT error path (although my taste 
> > would be to just append errtxt if errtxt!=NULL, which is less circumvent).
> > I also do not understand the changes to the call sites, where you add "%s". 
> > What is the point? If you want to separate error message and detail error 
> > message with " - ", you can just do an additional conditional strcat in the 
> > error message function.
> > Cheers, Thomas
> 
> Hi Thomas, sorry for the delay,
> 

why, there was no delay :-)

> Yes, the bug was the CRT errors being omitted, the issue with setting freeit 
> for CRT errors too is that it, from what I can see, is only supposed to be 
> used by the branch for Win32 errors, and is used for calling LocalFree 
> (required for FormatMessage) later on. Setting this for CRT errors would mean 
> LocalFree is passed something that LocalAlloc did not allocate, which would 
> likely result in bad things happening. 

Oh right. I see. Not your change, but I am not a big fan of this pattern 
(mixing CRT and win32 error handling). This is broken since it assumes that 
win32 error code and errno are mutually exclusive, or somehow related, which 
they are not. A lingering GetLastError() from some earlier win32 API call could 
be misreported as error detail for a follow-up CRT error. The correct way to do 
this would be to have two functions, one for failing win32 APIs, one for 
failing CRT functions. And let the caller decide what to call.

> I didn't feel like changing too much of the existing logic just to fix a 
> small issue and decided to leave the calculation and concatenation of the 
> message to other JLI functions, but that may have been a misjudgement on my 
> part.

If you can make the fix for the CRT extra info small, I'd go for it.

> 
> JLI_ReportErrorMessageSys is documented to deliberately leave the formatting 
> of spacing up to the caller - Which is why no changes were made to it to 
> accomodate that quirk. Admittedly a strcat could've been used in the caller 
> rather than an additional format specifer though, like you mentioned

I think it would be reasonable to expect that the additional info printed by 
that function gets visually separated somehow from the error text the caller 
hands in.

Note, btw, if you change output like this, make sure to check all test sources 
for tests that may grep the output. Not sure how likely this is here. Just 
something to keep in mind. This is nothing an IDE will tell you, unfortunately.

Cheers, Thomas

-

PR: https://git.openjdk.org/jdk/pull/9749


Re: RFR: 8291924: jpackage: l10n for Windows context menu label [v2]

2022-08-06 Thread Alexey Semenyuk
On Fri, 5 Aug 2022 17:04:54 GMT, Alex Kasko  wrote:

>> This change adds `ContextMenuCommandLabel` l10n property for file 
>> association context menu label. It is a follow-up to [this PR 
>> comment](https://github.com/openjdk/jdk/pull/9224#issuecomment-1169286082).
>> 
>> Note, non-EN l10n values were filled using auto-translator and may need to 
>> be corrected.
>> 
>> To check the contents of the label, registry query is added to 
>> `PackageTest::addHelloAppFileAssociationsVerifier`.
>> 
>> Testing:
>> 
>>  - [x] ran `FileAssociationsTest` with `install` enabled
>
> Alex Kasko has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reg method access, drop l10n entries, label check enhancement

I think there was a miscommunication about l10n files.
All "MsiInstallerStrings_*.wxl" files need to be updated, but no need to 
provide a localized version of English string(s). 
I.e. add `Open with 
[ProductName]` to all "MsiInstallerStrings_*.wxl" files, not only to 
"MsiInstallerStrings_en.wxl".

-

PR: https://git.openjdk.org/jdk/pull/9753


Re: RFR: 8291978: jpackage: allow to override primary l10n files on Windows

2022-08-06 Thread Alexey Semenyuk
On Fri, 5 Aug 2022 16:14:35 GMT, Alex Kasko  wrote:

> This change is a follow-up to [this 
> comment](https://bugs.openjdk.org/browse/JDK-8290519?focusedCommentId=14512038=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14512038).
> 
> Override implementation is based on [this 
> comment](https://bugs.openjdk.org/browse/JDK-8290519?focusedCommentId=14510886=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14510886).
> 
> As suggested in [this 
> comment](https://github.com/openjdk/jdk/pull/9753#pullrequestreview-1062560789)
>  only English translation is provided for new `resource.wxl-file` label.
> 
> `getTempdirectory` utility was moved from `BasicTest` to `WindowsHelper` to 
> be able to provide `--temp` and inspect the contents of `config` dir in 
> verifier. It is not clear if `WindowsHelper` is an appropriate place for this 
> utility (`BasicTest` is not Windows-specific), I've assumed that `--temp` 
> flag itself is only used on Windows.
> 
> Testing:
> 
>  - [x] enhanced `WinL10nTest` adding checks for `-loc` arguments to 
> `light.exe` and additional `@Parameter` run that overrides one of the primary 
> `.wxl` files
>  - [x] ran jtreg:jdk/tools/jpackage/windows and a `BasicTest` in unpack mode

Marked as reviewed by asemenyuk (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/9780


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Julian Waters
On Sat, 6 Aug 2022 11:36:44 GMT, Thomas Stuefe  wrote:

>> The only non Windows code that uses the `DLL_ERROR4` macro is Unix's 
>> java_md_common.c, where it reports the same `JVM_FindClassFromBootLoader` 
>> issue that Windows also does, and the shared args.c when a file cannot be 
>> found by path:
>> 
>> static JLI_List expandArgFile(const char *arg) {
>> JLI_List rv;
>> struct stat st;
>> FILE *fptr = fopen(arg, "r");
>> 
>> /* arg file cannot be opened */
>> if (fptr == NULL || fstat(fileno(fptr), ) != 0) {
>> JLI_ReportMessage(CFG_ERROR6, arg);
>> exit(1);
>> } else {
>> if (st.st_size > MAX_ARGF_SIZE) {
>> JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
>> exit(1);
>> }
>> }
>> 
>> rv = readArgFile(fptr);
>> 
>> /* error occurred reading the file */
>> if (rv == NULL) {
>> JLI_ReportMessage(DLL_ERROR4, arg);
>> exit(1);
>> }
>> fclose(fptr);
>> 
>> return rv;
>> }
>> 
>> 
>> In any case, the only thing that would change would be "Error: loading:" to 
>> "Error: Failed to load" as the prefix of the message, which semantically is 
>> the same whatever the context may be, just a small change I thought that 
>> would look better whenever the macro is used
>
> You also have one use where the VM does a GetProcAddress on windows. "Failed 
> to load" feels wrong for a failed dlsym. But "Error loading" is not much 
> better either, I admit.
> 
> In general, the smaller and focused you keep changes, the easier they are to 
> review and the better they are down-portable if that should be needed. I 
> usually try to do aesthetic changes in separate RFEs. In this change, if you 
> want to fix the display of additional diagnostic info, I'd just do that.

Agreed, will split this part into another RFE (initially didn't do this because 
such a change felt too trivial for an entire entry in the JBS), but I do feel 
like waiting for more opinions on what the prefix should be changed to before 
doing that

-

PR: https://git.openjdk.org/jdk/pull/9749


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Julian Waters
On Sat, 6 Aug 2022 10:54:56 GMT, Thomas Stuefe  wrote:

> Hi Julian,
> 
> I don't get it.
> 
> AFAICS the existing version displays the failure reason for win32 API just 
> fine, appending it to the message. Where is the bug? I see that the message 
> was omitted for CRT errors, so that is okay to fix, but that could have been 
> fixed with a simple freeit=1 in the CRT error path (although my taste would 
> be to just append errtxt if errtxt!=NULL, which is less circumvent).
> 
> I also do not understand the changes to the call sites, where you add "%s". 
> What is the point? If you want to separate error message and detail error 
> message with " - ", you can just do an additional conditional strcat in the 
> error message function.
> 
> Cheers, Thomas

Hi Thomas, sorry for the delay,

Yes, the bug was the CRT errors being omitted, the issue with setting freeit 
for CRT errors too is that it, from what I can see, is only supposed to be used 
by the branch for Win32 errors, and is used for calling LocalFree (required for 
FormatMessage) later on. Setting this for CRT errors would mean LocalFree is 
passed something that LocalAlloc did not allocate, which would likely result in 
bad things happening. I didn't feel like changing too much of the existing 
logic just to fix a small issue and decided to leave the calculation and 
concatenation of the message to other JLI functions, but that may have been a 
misjudgement on my part.

JLI_ReportErrorMessageSys is documented to deliberately leave the formatting of 
spacing up to the caller - Which is why no changes were made to it to 
accomodate that quirk. Admittedly a strcat could've been used in the caller 
rather than an additional format specifer though, like you mentioned

best regards,
Julian

-

PR: https://git.openjdk.org/jdk/pull/9749


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Thomas Stuefe
On Sat, 6 Aug 2022 11:14:55 GMT, Julian Waters  wrote:

>> src/java.base/share/native/libjli/emessages.h line 111:
>> 
>>> 109: #define DLL_ERROR2  "Error: failed %s, because %s"
>>> 110: #define DLL_ERROR3  "Error: could not find executable %s"
>>> 111: #define DLL_ERROR4  "Error: Failed to load %s"
>> 
>> Have you looked into all usages of this macro, also non-windows/shared code? 
>> Please make sure the replacement text makes sense in all those cases.
>
> The only non Windows code that uses the `DLL_ERROR4` macro is Unix's 
> java_md_common.c, where it reports the same `JVM_FindClassFromBootLoader` 
> issue that Windows also does, and the shared args.c when a file cannot be 
> found by path:
> 
> static JLI_List expandArgFile(const char *arg) {
> JLI_List rv;
> struct stat st;
> FILE *fptr = fopen(arg, "r");
> 
> /* arg file cannot be opened */
> if (fptr == NULL || fstat(fileno(fptr), ) != 0) {
> JLI_ReportMessage(CFG_ERROR6, arg);
> exit(1);
> } else {
> if (st.st_size > MAX_ARGF_SIZE) {
> JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
> exit(1);
> }
> }
> 
> rv = readArgFile(fptr);
> 
> /* error occurred reading the file */
> if (rv == NULL) {
> JLI_ReportMessage(DLL_ERROR4, arg);
> exit(1);
> }
> fclose(fptr);
> 
> return rv;
> }
> 
> 
> In any case, the only thing that would change would be "Error: loading:" to 
> "Error: Failed to load" as the prefix of the message, which semantically is 
> the same whatever the context may be, just a small change I thought that 
> would look better whenever the macro is used

You also have one use where the VM does a GetProcAddress on windows. "Failed to 
load" feels wrong for a failed dlsym. But "Error loading" is not much better 
either, I admit.

In general, the smaller and focused you keep changes, the easier they are to 
review and the better they are down-portable if that should be needed. I 
usually try to do aesthetic changes in separate RFEs. In this change, if you 
want to fix the display of additional diagnostic info, I'd just do that.

-

PR: https://git.openjdk.org/jdk/pull/9749


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Julian Waters
On Sat, 6 Aug 2022 10:36:05 GMT, Thomas Stuefe  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Missing spacing between errors
>
> src/java.base/share/native/libjli/emessages.h line 111:
> 
>> 109: #define DLL_ERROR2  "Error: failed %s, because %s"
>> 110: #define DLL_ERROR3  "Error: could not find executable %s"
>> 111: #define DLL_ERROR4  "Error: Failed to load %s"
> 
> Have you looked into all usages of this macro, also non-windows/shared code? 
> Please make sure the replacement text makes sense in all those cases.

The only non Windows code that uses the `DLL_ERROR4` macro is Unix's 
java_md_common.c, where it reports the same `JVM_FindClassFromBootLoader` issue 
that Windows also does, and the shared args.c when a file cannot be found by 
path:

static JLI_List expandArgFile(const char *arg) {
JLI_List rv;
struct stat st;
FILE *fptr = fopen(arg, "r");

/* arg file cannot be opened */
if (fptr == NULL || fstat(fileno(fptr), ) != 0) {
JLI_ReportMessage(CFG_ERROR6, arg);
exit(1);
} else {
if (st.st_size > MAX_ARGF_SIZE) {
JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
exit(1);
}
}

rv = readArgFile(fptr);

/* error occurred reading the file */
if (rv == NULL) {
JLI_ReportMessage(DLL_ERROR4, arg);
exit(1);
}
fclose(fptr);

return rv;
}


In any case, the only thing that would change would be "Error: loading:" to 
"Error: Failed to load" as the prefix of the message, which semantically is the 
same whatever the context may be, just a small change I thought that would look 
better whenever the macro is used

-

PR: https://git.openjdk.org/jdk/pull/9749


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Thomas Stuefe
On Thu, 4 Aug 2022 17:42:45 GMT, Julian Waters  wrote:

>> Please review a small patch for dumping the failure reason when the MSVCRT 
>> libraries or the Java Virtual Machine fails to load on Windows, which can 
>> provide invaluable insight when debugging related launcher issues. This also 
>> fixes a small bug in the Windows implementation of JLI_ErrorMessageSys where 
>> C runtime errors are never written to stderr.
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Missing spacing between errors

Hi Julian,

I don't get it.

AFAICS the existing version displays the failure reason for win32 API just 
fine, appending it to the message. Where is the bug? I see that the message was 
omitted for CRT errors, so that is okay to fix, but that could have been fixed 
with a simple freeit=1 in the CRT error path (although my taste would be to 
just append errtxt if errtxt!=NULL, which is less circumvent).

I also do not understand the changes to the call sites, where you add "%s". 
What is the point? If you want to separate error message and detail error 
message with " - ", you can just do an additional conditional strcat in the 
error message function.

Cheers, Thomas

src/java.base/share/native/libjli/emessages.h line 111:

> 109: #define DLL_ERROR2  "Error: failed %s, because %s"
> 110: #define DLL_ERROR3  "Error: could not find executable %s"
> 111: #define DLL_ERROR4  "Error: Failed to load %s"

Have you looked into all usages of this macro, also non-windows/shared code? 
Please make sure the replacement text makes sense in all those cases.

-

PR: https://git.openjdk.org/jdk/pull/9749


Re: RFR: 8291917: Windows - Dump error diagnostics when runtime libraries fail to load [v3]

2022-08-06 Thread Julian Waters
On Thu, 4 Aug 2022 17:42:45 GMT, Julian Waters  wrote:

>> Please review a small patch for dumping the failure reason when the MSVCRT 
>> libraries or the Java Virtual Machine fails to load on Windows, which can 
>> provide invaluable insight when debugging related launcher issues. This also 
>> fixes a small bug in the Windows implementation of JLI_ErrorMessageSys where 
>> C runtime errors are never written to stderr.
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Missing spacing between errors

Not entirely sure if that's the intended use of JLI_ReportErrorMessageSys, but 
since Windows doesn't actually use it normally, the fixing change to it should 
have minimal impact, if at all

-

PR: https://git.openjdk.org/jdk/pull/9749


Integrated: 8291954: Use Optional.isEmpty instead of !Optional.isPresent in java.base

2022-08-06 Thread Andrey Turbanov
On Mon, 1 Aug 2022 06:51:44 GMT, Andrey Turbanov  wrote:

> I propose to replace usages of !Optional.isPresent() with Optional.isEmpty 
> method.
> It's makes code a bit easier to read.
> Noticing negation before long chain of method calls is hard.

This pull request has now been integrated.

Changeset: ae520537
Author:Andrey Turbanov 
URL:   
https://git.openjdk.org/jdk/commit/ae52053757ca50c4b56989c9b0c6890e504e4088
Stats: 23 lines in 6 files changed: 0 ins; 2 del; 21 mod

8291954: Use Optional.isEmpty instead of !Optional.isPresent in java.base

Reviewed-by: jpai, alanb, lancea, rriggs, bpb

-

PR: https://git.openjdk.org/jdk/pull/9697


More elegant multi-condition groupby. #9719

2022-08-06 Thread (11I10I24) ?? ?? ?? ??
Hi, team


I have two suggestions


Firstly:


If we can provide a new API for grouping (java.util.stream.Collectors)
such as:
userList.stream().collect(Collectors.groupingBy(User::getName, User::getCity))
userList.stream().collect(Collectors.groupingBy("-", User::getName, 
User::getCity, User::getId));// key is "name-city", value isList after 
being grouped


It helps us to do grouping with less code. andIt looks cooler for lambda.


AndI've provided a simple demo.
demo:https://github.com/1015770492/CollectorsDemo/blob/master/src/main/java/com/example/demo/MultiGroupByDemo.java
issue: https://github.com/openjdk/jdk/pull/9719


Secondly:


similarly, if we can provide new api for sort with stream.
It can be??
userList.stream().sorted(User::getAge)
oruserList.stream().sorted(User::getName)
oruserList.stream().sorted(User::getName, User::getAge)


userList.stream().sorted(User::getAge) age is a Integer or int type ,it 
isComparable, So if we can use theFunction 
referenceimplements sort.


String implements theComparable Interface but we do not use 
thefeatures.




userList.stream().sorted(User::getCity, User::getName)

multi-condition sort we can transfor to String sort.




best regards
Yu, Jinhua
1015770...@qq.com

Re: [jdk19] RFR: 8278274: Update nroff pages in JDK 19 before RC

2022-08-06 Thread Patrick Pfeifer
On Mon, 18 Jul 2022 15:22:01 GMT, Jonathan Gibbons  wrote:

>> Please review these changes to the nroff manpage files so that they match 
>> their markdown sources that Oracle maintains.
>> 
>> All pages at a minimum have 19-ea replaced with 19, and copyright set to 
>> 2022 if needed.  Additionally:
>> 
>> The Java manpage was missing updates from:
>> - [JDK-8282018](https://bugs.openjdk.org/browse/JDK-8282018): Add captions 
>> to tables on java man page.
>> 
>> The Java manpage has slight formatting differences from:
>> - [JDK-8262004](https://bugs.openjdk.org/browse/JDK-8262004): Classpath 
>> separator: Man page says semicolon; should be colon on Linux
>> - [JDK-8236569](https://bugs.openjdk.org/browse/JDK-8236569): -Xss not 
>> multiple of 4K does not work for the main thread on macOS
>> 
>> The Java manpage has a typo fixed in mainline by 
>> [JDK-8279047](https://bugs.openjdk.org/browse/JDK-8279047) (for JDK 20)
>> 
>> 
>> The keytool manpage was missing updates from:
>> - [JDK-8282014](https://bugs.openjdk.org/browse/JDK-8282014): Add captions 
>> to tables on keytool man page.
>> - [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
>> default key sizes and algorithms based on CNSA
>> 
>> The jar manpage was missing updates from:
>> - [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod 
>> man pages need the new --date documenting from CSR 
>> [JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)
>> 
>> The jarsigner manpage was missing updates from:
>> - [JDK-8282015](https://bugs.openjdk.org/browse/JDK-8282015): Add captions 
>> to tables on jarsigner man page.
>> - [JDK-8267319](https://bugs.openjdk.org/browse/JDK-8267319): Use larger 
>> default key sizes and algorithms based on CNSA
>> 
>> The javadoc manpage was missing updates from:
>> - [JDK-8279034](https://bugs.openjdk.org/browse/JDK-8279034): Update man 
>> page for javadoc `--date` option
>> 
>> The jmod manpage was missing updates from:
>> - [JDK-8278764](https://bugs.openjdk.org/browse/JDK-8278764): jar and jmod 
>> man pages need the new --date documenting from CSR 
>> [JDK-8277755](https://bugs.openjdk.org/browse/JDK-8277755)
>> 
>> The jpackage manpage was missing updates from:
>> - [JDK-8285146](https://bugs.openjdk.org/browse/JDK-8285146): Document 
>> jpackage resource dir feature
>> - [JDK-8284695](https://bugs.openjdk.org/browse/JDK-8284695): Update 
>> jpackage man pages for JDK 19
>> - [JDK-8284209](https://bugs.openjdk.org/browse/JDK-8284209): Replace 
>> remaining usages of 'a the' in source code
>> 
>> The jshell manpage was missing updates from:
>> - [JDK-8282016](https://bugs.openjdk.org/browse/JDK-8282016): Add captions 
>> to tables on jshell man page.
>
> src/java.base/share/man/keytool.1 line 456:
> 
>> 454: \f[CB]PrivateKeyEntry\f[R] for the signer that already exists in the
>> 455: keystore.
>> 456: This option is used to sign the certificate with the signer?s private
> 
> Not a problem with this PR as such, but we still have a `?` character in the 
> output.

Hello @jonathan-gibbons

Excuse my hijacking / piggy-backing on this conversation!

When you say

> Not a problem with this PR as such, but we still have a `?` character in the 
> output.

This strongly suggests that there must be an input to those man pages 
somewhere. :-) Would you mind to point me to it? Are they even included in the 
git repo?

Also, I was wondering why these man pages (i.e. the outputs) are obviously not 
included any more in the OpenJDK builds, e.g. https://jdk.java.net/19/ ? I am 
not sure when they were removed or if they were ever included in _open_jdk 
builds at all, but I see them in the Oracle JDK 1.8 Build.

I find them really nice to read! Are there other output formats, e.g. HTML, 
that might be are deployed somewhere on the web?

-

PR: https://git.openjdk.org/jdk19/pull/145


Re: RFR: 8290036: Define and specify Runtime shutdown sequence [v4]

2022-08-06 Thread Alan Bateman
On Sat, 6 Aug 2022 00:42:23 GMT, Stuart Marks  wrote:

>> Initial edits to addShutdownHook from Alex.
>> 
>> See [JDK-8290036](https://bugs.openjdk.org/browse/JDK-8290036).
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   More edits from Alex's suggestions.

src/java.base/share/classes/java/lang/System.java line 1888:

> 1886:  * Initiates the shutdown 
> sequence of the Java Virtual
> 1887:  * Machine. This method always blocks indefinitely. The
> 1888:  * argument serves as a status code; by convention, a nonzero status

Would you remind reflowing this one too as the latest change makes mixes the 
line lengths again.

src/java.base/share/classes/java/lang/Thread.java line 73:

> 71:  * or if its {@code run} method completes abruptly and the appropriate
> 72:  * {@linkplain Thread.UncaughtExceptionHandler uncaught exception 
> handler} completes
> 73:  * normally or abruptly. With no code left to run, the thread has 
> completed execution.

I'm in two minds about introducing the UHE in paragraph 3 but probably okay as 
you kinda have to mention it when defining termination. There's a lot more to 
this story when you have agents in the picture but I'll not go there. I wonder 
if we can do something with the last sentence that includes the word 
"terminated". We use "terminated" in several places and it would be cleared if 
this sentence were to end with something like "... the thread has completes 
execution, and is terminated" (it could link to isAlive or 
Thread.State#TERMINATED).

Given that the join method is introduced as the method to wait for a thread to 
terminate then it could be part of this paragraph rather than a single sentence 
paragraph.

src/java.base/share/classes/java/lang/Thread.java line 104:

> 102:  * The shutdown sequence begins when 
> all started
> 103:  * non-daemon threads have terminated. Unstarted non-daemon threads do 
> not prevent
> 104:  * the shutdown sequence from commencing. Invoking the {@linkplain 
> Runtime#exit(int)}

I think this is the only usage of "commencing", everywhere else uses 
"beginning" or "begin the shutdown sequence".

-

PR: https://git.openjdk.org/jdk/pull/9437


RFR: 8290041: ModuleDescriptor.hashCode is inconsistent

2022-08-06 Thread Jaikiran Pai
Can I please get a review of this change which proposes to fix 
https://bugs.openjdk.org/browse/JDK-8290041? 

As noted by the reporter, the current implementation is buggy since the 
calculation can result in a different value of the hashcode depending on the 
order of iteration of the `Modifier`s. The commit in this PR changes that 
computation to produce consistent result irrespective of the order in which the 
`Modifier`s (enum) is iterated upon.

A new test has been added which reproduces the issue and verifies the fix.

-

Commit messages:
 - 8290041: ModuleDescriptor.hashCode is inconsistent

Changes: https://git.openjdk.org/jdk/pull/9790/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=9790=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290041
  Stats: 47 lines in 2 files changed: 41 ins; 3 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/9790.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9790/head:pull/9790

PR: https://git.openjdk.org/jdk/pull/9790