[jdk19] Integrated: 8289558: Need spec clarification of j.l.foreign.*Layout

2022-07-07 Thread Maurizio Cimadamore
On Fri, 1 Jul 2022 11:03:23 GMT, Maurizio Cimadamore  
wrote:

> This patch fixes few javadoc issues in the memory layout API.
> The main issues are that `SequenceLayout::flatten` and 
> `SequenceLayout::reshape` still mention failures caused by a lack of size. 
> But that condition is no longer possible in the new API.
> 
> The javadoc of `ValueLayout::arrayElementVarHandle` is suboptimal and can be 
> clarified - UOE is only thrown if the value layout alignment is bigger than 
> its size.
> 
> Finally, the `MemoryLayout::equals` method does not mention value layout 
> carriers.
> 
> The JBS issue associated with this PR mentions also other issues, mostly 
> related to the overly broad visibility of some of the methods in the javadoc 
> (e.g. isPadding). Unfortunately, given the presence of an intermediate, 
> non-public, abstract class, this is what we get from javadoc. Fixing these 
> issues would require a deeper restructuring of the implementation, which 
> would be too riskt at this stage.

This pull request has now been integrated.

Changeset: 889150b4
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.org/jdk19/commit/889150b47a7a33d302c1883320d2cfbb915c52e7
Stats: 48 lines in 5 files changed: 14 ins; 11 del; 23 mod

8289558: Need spec clarification of j.l.foreign.*Layout

Reviewed-by: psandoz, jvernee

-

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


Re: [jdk19] RFR: 8289223: Canonicalize header ids in foreign API javadocs

2022-07-07 Thread Maurizio Cimadamore
On Mon, 27 Jun 2022 16:44:17 GMT, Jorn Vernee  wrote:

> https://github.com/openjdk/jdk/pull/8817 added a button to copy a link to a 
> section of javadoc to the clipboard. For the copy button to appear, the 
> header needs to have an `id`.
> 
> This cleanup PR canonicalizes all header ids in the java.lang.foreign package 
> to the preferred (non-legacy) style, and adds ids in places where they are 
> missing as well.
> 
> In accordance with the Late-Enhancement Request Process [1], this is a 
> `noreg-doc` documentation only change, which does not require an enhancement 
> request.
> 
> [1]: https://openjdk.org/jeps/3#Late-Enhancement-Request-Process

Looks good

-

Marked as reviewed by mcimadamore (Reviewer).

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]

2022-07-07 Thread Stuart Marks
> Simple javadoc fix of an editorial nature.

Stuart Marks has updated the pull request incrementally with one additional 
commit since the last revision:

  Reflow and adjust whitespace per Alan's & Iris' comments.

-

Changes:
  - all: https://git.openjdk.org/jdk19/pull/111/files
  - new: https://git.openjdk.org/jdk19/pull/111/files/8902f466..a3fbdb5b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk19=111=01
 - incr: https://webrevs.openjdk.org/?repo=jdk19=111=00-01

  Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk19/pull/111.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/111/head:pull/111

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


[jdk19] RFR: 8289872: wrong wording in @param doc for HashMap.newHashMap et. al.

2022-07-07 Thread Stuart Marks
Minor doc wording changes, to be consistent with HashSet.newHashSet et. al.

-

Commit messages:
 - 8289872: wrong wording in @param doc for HashMap.newHashMap et. al.

Changes: https://git.openjdk.org/jdk19/pull/118/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=118=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289872
  Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk19/pull/118.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/118/head:pull/118

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


RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[]

2022-07-07 Thread Сергей Цыпанов
We can skip bounds check and null check for Charset in case we use the array 
entirely and the Charset is either default one or proven to be non-null.

Benchmark results:

before

Benchmark  Mode  Cnt  Score   
Error  Units
StringConstructor.newStringFromArray   avgt   50  4,815 ± 
0,154  ns/op
StringConstructor.newStringFromArrayWithCharsetavgt   50  4,462 ± 
0,068  ns/op
StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,653 ± 
0,040  ns/op
StringConstructor.newStringFromRangedArray avgt   50  5,090 ± 
0,066  ns/op
StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,550 ± 
0,041  ns/op
StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  8,080 ± 
0,055  ns/op

after

Benchmark  Mode  Cnt  Score   
Error  Units
StringConstructor.newStringFromArray   avgt   50  4,595 ± 
0,053  ns/op
StringConstructor.newStringFromArrayWithCharsetavgt   50  4,038 ± 
0,062  ns/op
StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,035 ± 
0,031  ns/op
StringConstructor.newStringFromRangedArray avgt   50  4,084 ± 
0,007  ns/op
StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,014 ± 
0,008  ns/op
StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  7,466 ± 
0,071  ns/op

-

Commit messages:
 - 8289908: Skip bounds check for cases when String is constructed from 
entirely used byte[]

Changes: https://git.openjdk.org/jdk/pull/9407/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=9407=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289908
  Stats: 90 lines in 5 files changed: 77 ins; 1 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/9407.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9407/head:pull/9407

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses

2022-07-07 Thread Alan Bateman
On Wed, 6 Jul 2022 00:32:00 GMT, Stuart Marks  wrote:

> Simple javadoc fix of an editorial nature.

src/java.base/share/classes/java/util/Map.java line 745:

> 743:  * ( href="{@docRoot}/java.base/java/util/Collection.html#optional-restrictions">optional)
> 744:  * @throws NullPointerException if the specified function is null, 
> or if a replacement value is null
> 745:  * and this map does not permit null values

This looks okay although I'd probably reflow L744-745 to avoid to keep the line 
lengths somewhat consistent with the other exceptions.

-

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


Re: RFR: 8288838: jpackage: file association additional arguments [v4]

2022-07-07 Thread Alexander Matveev
On Mon, 4 Jul 2022 23:38:08 GMT, Alex Kasko  wrote:

>> jpackage implementation of file association on Windows currently passes a 
>> selected filename as an only argument to associated executable.
>> 
>> It is proposed to introduce additional option in file association property 
>> file to allow optionally support additional arguments using `%*` batch 
>> wildcard.
>> 
>> Note, current implementation, while fully functional, is only a **DRAFT** 
>> one, it is not ready for integration in this form. I would appreciate any 
>> guidance on the following points:
>> 
>>  - option naming inside a properties file, currently `pass-all-args` is used
>>  - option naming in a bundler parameter implementation, it is not clear if 
>> it should introduce a new group of "file association windows specific 
>> options" next to the existing "file association mac specific options" group
>>  - test organization to cover the new option: currently it is included 
>> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) 
>> `includeDescription` parameter; it is not clear whether it should be done in 
>> a separate test and whether to include runs for every parameter combination
>>  - test run implementation: currently arguments are checked when a file with 
>> associated extension is invoked from command line; it is not clear whether 
>> it would be more appropriate instead to create a desktop shortcut with the 
>> same command as a target and to invoke it with `java.awt.Desktop`
>> 
>> Also please note, that full install/uninstall run is currently enabled in 
>> `FileAssociationTest`, it is intended to be used only in a draft code during 
>> the development and to be removed (to use the same "install or unpack" logic 
>> as other tests) in a final version.
>> 
>> Testing:
>>  
>> - [x] test to cover new logic is included
>> - [x] ran jtreg:jdk/tools/jpackage with no new failures
>
> Alex Kasko has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains three commits:
> 
>  - enable passing args, add test coverage
>  - Added input params validation
>  - Proposed test changes to make FA testing of jpackage more flexible

Marked as reviewed by almatvee (Reviewer).

-

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


RFR: 8289797: [TESTBUG]tools/launcher/I18NArgTest.java fails on Japanese Windows enviromnent

2022-07-07 Thread KIRIYAMA Takuya
I removed a section of via JDK_JAVA_OPTIONS because including main class is not 
allowed in the specification.
This behavior is added in JDK-8170832, which add JAVA_OPTIONS environment 
variable. At this time, this test is mismatch with the specification.
I tried to test and get Passed on Japanese Windows environment.
Could you review this fix, please?

-

Commit messages:
 - 8289797: [TESTBUG]tools/launcher/I18NArgTest.java fails on Japanese Windows 
enviromnent

Changes: https://git.openjdk.org/jdk/pull/9389/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=9389=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289797
  Stats: 15 lines in 1 file changed: 0 ins; 14 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/9389.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9389/head:pull/9389

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


Re: RFR: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets

2022-07-07 Thread Joe Darcy
On Wed, 6 Jul 2022 20:49:25 GMT, John R Rose  wrote:

> The code examples in these files were exdented to make it easier to extract 
> example code and test it, and to maintain equivalence during active 
> development of the APIs.
> 
> The examples in runnable form are in 
> test/jdk/java/lang/invoke/JavaDocExamplesTest.java in the same repo.
> 
> It would be nice if some day we could tie together the two copies into one 
> using a link. I don't recall whether snippets can do this or not, given that 
> the two copies are far apart in the repo structure.
> 
> I agree that just the bracket changes are valuable by themselves. I also have 
> no objection to fixing the exdentation.

Thanks John; I'll push the update to use the snippet tag as-is and leave 
further refinements to future work.

-

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


Re: RFR: 8279283 - BufferedInputStream should override transferTo [v5]

2022-07-07 Thread Сергей Цыпанов
On Mon, 27 Dec 2021 13:43:12 GMT, Markus KARG  wrote:

>> Implementation of JDK-8279283
>
> Markus KARG has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixed missing BufferedInputStream

src/java.base/share/classes/java/io/BufferedInputStream.java line 495:

> 493: int avail = count - pos;
> 494: if (avail > 0) {
> 495: out.write(buf, pos, avail);

I think `buf` should not be accessed directly but via `getBufIfOpen()` because 
we need to throw IOE in case `this` is closed.

-

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


Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]

2022-07-07 Thread Alexander Matveev
> Fixed 3 issues which made signature invalid:
> - We should not remove .jpackage.xml from signed app image when creating DMG 
> or PKG otherwise it invalidates signature.
> - .package should be created when app image is generated, so this file can be 
> signed.
> - Copying predefine app image for DMG and PKG should not follow symbolic 
> links, otherwise several files from runtime (COPYRIGHT and LICENSE) will be 
> copied instead of symbolic links being created, since it invalidates 
> signature as well.
> 
> Added additional test to validate signature when DMG or PKG is generated from 
> predefined app image.

Alexander Matveev 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 three additional 
commits since the last revision:

 - Merge remote-tracking branch 'upstream/master' into JDK-8289030
 - 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
 - 8289030: [macos] app image signature invalid when creating DMG or PKG

-

Changes:
  - all: https://git.openjdk.org/jdk19/pull/89/files
  - new: https://git.openjdk.org/jdk19/pull/89/files/5e47a1e3..4d5d4bce

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk19=89=01
 - incr: https://webrevs.openjdk.org/?repo=jdk19=89=00-01

  Stats: 1986 lines in 64 files changed: 1671 ins; 95 del; 220 mod
  Patch: https://git.openjdk.org/jdk19/pull/89.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/89/head:pull/89

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


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v14]

2022-07-07 Thread Alan Bateman
On Thu, 30 Jun 2022 16:41:36 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   * Fix @since version

This PR has been open and in development for a long time. 

One question is whether Arrays.sort has reached its "complexity budget". Right 
now, and before the addition of radix sort, there are cases where it will do 
insertion, counting, or heap sorts. Laurent has provided a good set of results 
to support the case for using radix for random data but it does beg the 
question on whether Array.sort really needs to try to be optimal for all cases, 
esp. as there are 4 implementations of each for int, long, float and double 
elements.

Another question is whether the space/performance of tradeoff of using radix 
sort is too much of a change for Arrays.sort. If I read it correctly then it 
may allocate up to 1/128 of the heap but I can't immediately see the 
implications for parallel sort and whether the % of the heap used may be 
several allocations to up this size (it looks like it is only done on the 
non-left parts but I'm not 100% sure yet). I see the OOME handling so it can 
continue when it's not possible to allocate but it may be that allocating big 
arrays during sort will cause something else in the system to OOME.

So before going any further, I think it would be useful to get input on whether 
this large change has support or whether there are suggestions for other 
avenues of exploration before committing to the proposal here.

-

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]

2022-07-07 Thread Pavel Rappo
On Wed, 6 Jul 2022 23:03:42 GMT, Stuart Marks  wrote:

>> Simple javadoc fix of an editorial nature.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflow and adjust whitespace per Alan's & Iris' comments.

Marked as reviewed by prappo (Reviewer).

-

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


Re: RFR: JDK-8289106: Add model of class file versions to core reflection [v2]

2022-07-07 Thread Adam Sotona
On Wed, 29 Jun 2022 21:32:29 GMT, Joe Darcy  wrote:

>> JDK-8289106: Add model of class file versions to core reflection
>
> Joe Darcy 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 10 additional commits since 
> the last revision:
> 
>  - Add method to map from major class file version.
>  - Merge branch 'master' into JDK-8289106
>  - Update phrasing.
>  - Augment and correct docs.
>  - Correct major versions; RELEASE_0 and RELEASE_1 are both 45.
>  - Make major method public.
>  - Add AccessFlag.locations(ClassFileFormatVersion)
>  - Add ClassFileFormatVersion <-> Runtime.Version conversions.
>  - Merge branch 'master' into JDK-8289106
>  - JDK-8289106: Add model of class file versions to core reflection

I like the proposed concept and Classfile Processing API is definitely one of 
the potential consumers.

-

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


Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]

2022-07-07 Thread Maurizio Cimadamore
On Wed, 6 Jul 2022 17:07:37 GMT, Jorn Vernee  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert implicit vs. heap session changes
>
> src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/macos/MacOsAArch64VaList.java
>  line 172:
> 
>> 170: 
>> 171: public Builder(MemorySession session) {
>> 172: ((MemorySessionImpl)session).checkValidState();
> 
> Or here, if the memory session is a non-closeable view.

I believe there was a wrong renaming with the IDE here, I will fix this

-

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


Re: RFR: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets

2022-07-07 Thread John R Rose
On Tue, 5 Jul 2022 22:22:46 GMT, Joe Darcy  wrote:

> Update existing examples in  java.lang.invoke.{MethodHandle, MethodHandles} 
> to use snippets rather than the older markup idiom.

The code examples in these files were exdented to make it easier to extract 
example code and test it, and to maintain equivalence during active development 
of the APIs.

The examples in runnable form are in 
test/jdk/java/lang/invoke/JavaDocExamplesTest.java in the same repo.

It would be nice if some day we could tie together the two copies into one 
using a link.  I don't recall whether snippets can do this or not, given that 
the two copies are far apart in the repo structure.

I agree that just the bracket changes are valuable by themselves.  I also have 
no objection to fixing the exdentation.

-

Marked as reviewed by jrose (Reviewer).

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


Re: RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets

2022-07-07 Thread Alan Bateman
On Wed, 6 Jul 2022 16:18:08 GMT, Ichiroh Takiguchi  
wrote:

> Discussions are available on :
> [JDK-8289834](https://bugs.openjdk.org/browse/JDK-8289834): Add SBCS and DBCS 
> Only EBCDIC charsets

Yes, I think this need discussion on whether the JDK really needs to keep 
including and adding more EBCDIC charsets. I understand they can be useful for 
someone using JDBC to connect to a database on z/OS but this scenario would 
work equally well if the EBCDIC charsets were deployed on the class path or 
module path. Do you know if the icu4j project is still alive? I've often 
wondered why there wasn't more use of the provider mechanism.

-

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]

2022-07-07 Thread Iris Clark
On Wed, 6 Jul 2022 23:03:42 GMT, Stuart Marks  wrote:

>> Simple javadoc fix of an editorial nature.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflow and adjust whitespace per Alan's & Iris' comments.

Marked as reviewed by iris (Reviewer).

-

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


Re: [jdk19] RFR: 8289601: SegmentAllocator::allocateUtf8String(String str) should be clarified for strings containing \0 [v2]

2022-07-07 Thread Maurizio Cimadamore
On Wed, 6 Jul 2022 11:09:33 GMT, Jorn Vernee  wrote:

>> This PR updates the spec and implementation to throw an 
>> `IllegalArgumentException` when an attempt is made to convert a Java string 
>> containing null characters to a C string.
>> 
>> Testing: local run of the `jdk_foreign` test suite.
>
> Jorn Vernee has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add javadoc
>  - Revert "Throw IAE when converting string with null byte."
>
>This reverts commit 70f83902a38e863b96fedb312982f7ac683ed068.

Looks good

-

Marked as reviewed by mcimadamore (Reviewer).

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


Re: RFR: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets

2022-07-07 Thread Joe Darcy
On Wed, 6 Jul 2022 00:08:04 GMT, liach  wrote:

>> Update existing examples in  java.lang.invoke.{MethodHandle, MethodHandles} 
>> to use snippets rather than the older markup idiom.
>
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 272:
> 
>> 270:  * Here are some examples of usage:
>> 271:  * {@snippet lang="java" :
>> 272: Object x, y; String s; int i;
> 
> These lines can now be properly indented, and they should be.

That isn't an unreasonable suggestion. However, I wanted to present this change 
in an easy-to-review fashion so limited the diffs to the bracketing tags.

-

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


Re: RFR: 8289768: Clean up unused code [v2]

2022-07-07 Thread Naoto Sato
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński  wrote:

>> This patch removes many unused variables and one unused label reported by 
>> the compilers when relevant warnings are enabled. 
>> 
>> The unused code was found by compiling after removing `unused` from the list 
>> of disabled warnings for 
>> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190)
>>  and 
>> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203),
>>  and enabling 
>> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170)
>>  MSVC warning.
>> 
>> I only removed variables that were uninitialized or initialized without side 
>> effects. I verified that the removed variables were not used in any 
>> `#ifdef`'d code. I checked that the changed code still compiles on Windows, 
>> Linux and Mac, both in release and debug versions.
>
> Daniel Jeliński 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 four additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin' into unused-variables
>  - Remove unused variables (windows)
>  - Remove unused variables (macos)
>  - Remove unused variables (linux)

Marked as reviewed by naoto (Reviewer).

-

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


Re: [jdk19] RFR: 8289872: wrong wording in @param doc for HashMap.newHashMap et. al.

2022-07-07 Thread Chris Hegarty
On Thu, 7 Jul 2022 06:06:42 GMT, Stuart Marks  wrote:

> Minor doc wording changes, to be consistent with HashSet.newHashSet et. al.

LGTM

-

Marked as reviewed by chegar (Reviewer).

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


Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v5]

2022-07-07 Thread Maurizio Cimadamore
On Wed, 6 Jul 2022 18:01:28 GMT, Maurizio Cimadamore  
wrote:

>> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge branch 'master' into memory_session_cleanup
>  - Fix ambiguity between session vs. base session
>  - Revert implicit vs. heap session changes
>  - Unify heap vs. implicit scopes
>  - Merge branch 'master' into memory_session_cleanup
>  - Fix issue in Direct-X-Buffer template
>  - Simplify and drop the state class
>  - Add missing files
>  - Initial push

I've fixed the issues in the review comments, and further reduced the 
differences between the contents of this patch and mainline (while retaining 
the improvements).

-

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


Re: RFR: 8288838: jpackage: file association additional arguments [v3]

2022-07-07 Thread Alex Kasko
On Thu, 30 Jun 2022 18:50:23 GMT, Alexey Semenyuk  wrote:

>> Alex Kasko has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   drop pass-all-args property
>
> test/jdk/tools/jpackage/share/FileAssociationsTest.java line 108:
> 
>> 106: .applyTo(packageTest);
>> 107: 
>> 108: packageTest.run(RunnablePackageTest.Action.CREATE, 
>> RunnablePackageTest.Action.INSTALL,
> 
> FYI: the default test steps can be overridden with the value of 
> `jpackage.test.action` system property.
> Its value would be `create,install,verify_install,uninstall` for this case.
> 
> UPD: Nevermind this comment. It applied to the old version of the PR.

Thanks for the info! I've seen `jpackage.test.action` property in 
`run_tests.sh`, but never used it.

-

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


RFR: 8249834: Marked Bug8146568 and Bug8148174 as manual tests.

2022-07-07 Thread Bill Huang
Tests Bug8146568 and Bug8148174 were disabled for high memory consumption, over 
17G. This is a task to re-enable these two tests by marking them as manual 
tests.

-

Commit messages:
 - Marked Bug8146568 and Bug8148174 as manual tests.

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

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


Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v6]

2022-07-07 Thread Maurizio Cimadamore
> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Turn non-closeable view back into MemorySession impl

-

Changes:
  - all: https://git.openjdk.org/jdk19/pull/22/files
  - new: https://git.openjdk.org/jdk19/pull/22/files/09bb7cf3..809a0a2e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk19=22=05
 - incr: https://webrevs.openjdk.org/?repo=jdk19=22=04-05

  Stats: 79 lines in 14 files changed: 7 ins; 11 del; 61 mod
  Patch: https://git.openjdk.org/jdk19/pull/22.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/22/head:pull/22

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


Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]

2022-07-07 Thread Stuart Marks
On Wed, 1 Jun 2022 16:45:35 GMT, liach  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   do it as naotoj said
>
> 'the new' fix should be applied to newHashMap etc. too.

@liach 

> 'the new' fix should be applied to newHashMap etc. too.

I've filed [JDK-8289872](https://bugs.openjdk.org/browse/JDK-8289872) for this. 
It's a small thing but best to fix it before we forget about it.

-

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


Re: RFR: 8288838: jpackage: file association additional arguments [v3]

2022-07-07 Thread Alexey Semenyuk
On Wed, 29 Jun 2022 12:29:33 GMT, Alex Kasko  wrote:

>> jpackage implementation of file association on Windows currently passes a 
>> selected filename as an only argument to associated executable.
>> 
>> It is proposed to introduce additional option in file association property 
>> file to allow optionally support additional arguments using `%*` batch 
>> wildcard.
>> 
>> Note, current implementation, while fully functional, is only a **DRAFT** 
>> one, it is not ready for integration in this form. I would appreciate any 
>> guidance on the following points:
>> 
>>  - option naming inside a properties file, currently `pass-all-args` is used
>>  - option naming in a bundler parameter implementation, it is not clear if 
>> it should introduce a new group of "file association windows specific 
>> options" next to the existing "file association mac specific options" group
>>  - test organization to cover the new option: currently it is included 
>> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) 
>> `includeDescription` parameter; it is not clear whether it should be done in 
>> a separate test and whether to include runs for every parameter combination
>>  - test run implementation: currently arguments are checked when a file with 
>> associated extension is invoked from command line; it is not clear whether 
>> it would be more appropriate instead to create a desktop shortcut with the 
>> same command as a target and to invoke it with `java.awt.Desktop`
>> 
>> Also please note, that full install/uninstall run is currently enabled in 
>> `FileAssociationTest`, it is intended to be used only in a draft code during 
>> the development and to be removed (to use the same "install or unpack" logic 
>> as other tests) in a final version.
>> 
>> Testing:
>>  
>> - [x] test to cover new logic is included
>> - [x] ran jtreg:jdk/tools/jpackage with no new failures
>
> Alex Kasko has refreshed the contents of this pull request, and previous 
> commits have been removed. Incremental views are not available.

test/jdk/tools/jpackage/share/FileAssociationsTest.java line 108:

> 106: .applyTo(packageTest);
> 107: 
> 108: packageTest.run(RunnablePackageTest.Action.CREATE, 
> RunnablePackageTest.Action.INSTALL,

FYI: the default test steps can be overridden with the value of 
`jpackage.test.action` system property.
Its value would be `create,install,verify_install,uninstall` for this case.

UPD: Nevermind this comment. It applied to the old version of the PR.

-

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses

2022-07-07 Thread Iris Clark
On Wed, 6 Jul 2022 00:32:00 GMT, Stuart Marks  wrote:

> Simple javadoc fix of an editorial nature.

Marked as reviewed by iris (Reviewer).

src/java.base/share/classes/java/util/Map.java line 740:

> 738:  * @param function the function to apply to each entry
> 739:  * @throws UnsupportedOperationException if the {@code set} operation
> 740:  * is not supported by this map's entry set iterator.

Maybe fix indentation here and at line 742 for the CCE for consistency?

-

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


Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]

2022-07-07 Thread David Holmes
On Tue, 5 Jul 2022 21:56:27 GMT, Kim Barrett  wrote:

> I think that isn't true. If a hook doesn't terminate then VM.shutdown doesn't 
> get called, so the window never gets cracked opened to call halt directly.

@kimbarrett  Any thread can call halt() directly while the attempted shutdown 
is in progress.

-

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


Re: RFR: 8289711: Add container configuration data to mbeans [v2]

2022-07-07 Thread Severin Gehwolf
On Wed, 6 Jul 2022 03:52:30 GMT, xpbob  wrote:

>> Container configuration information is useful for troubleshooting 
>> problems,Exposing information in MBeans is ideal for monitoring, jConsole, 
>> and other scenarios.
>> Results the following
>> ![图片](https://user-images.githubusercontent.com/7837910/177248834-50beefe9-4db6-470c-8f15-df5a93892804.png)
>
> xpbob has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   update header

I do think this could be useful.

Having said that, some of this has been discussed in 
https://bugs.openjdk.org/browse/JDK-8199944 (and been considered as won't fix 
at the time). I tend to agree that it should be a JDK (and even an OS) specific 
bean. The relevant information is only available on Linux. +1 on making this 
JDK specific. First step is probably to get consensus on a CSR.

-

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


Re: RFR: 8288984: Simplification in Shutdown.exit [v2]

2022-07-07 Thread David Holmes
On Tue, 5 Jul 2022 04:51:50 GMT, Ryan Ernst  wrote:

>> YAO (Yet Another Opinion)!  But I think we're actually converging.
>> 
>> David's wording:
>>>Invocations of this method are serialized such that only one invocation will 
>>>actually proceed with the shutdown sequence and terminate the VM with the 
>>>given status code.
>> 
>> ... gives the impression that an invocation of this method WILL terminate 
>> the VM with the given status code - which is not actually true, given the 
>> potential for signals. This is already alluded to in 
>> `Runtime::addShutdownHook`. I agree that this could be resolved separately, 
>> but we should _allow_ for signals (even if we don't explicitly mention them).
>>  
>> The original wording:
>>> If this method is invoked after all shutdown hooks have already been run 
>>> ... 
>> 
>> ... seems quite clever (and allows for signals). Maybe we could keep things 
>> super simple:
>>> If this method is invoked after shutdown hooks have already been started, 
>>> it will block indefinitely. If this method is invoked from a shutdown hook 
>>> the system will deadlock.
>
> I like the new suggested wording, but I would slightly tweak it. As Alan 
> mentioned in another comment the first paragraph makes clear the method never 
> returns normally. How does the following sound?
> 
>> If this method is invoked after shutdown hooks have already been started, 
>> the supplied status code will be ignored. If this method is invoked from a 
>> shutdown hook the system will deadlock.

> gives the impression that an invocation of this method WILL terminate the VM 
> with the given status code - which is not actually true, given the potential 
> for signals.

The subtle distinction being calling Runtime.exit versus Shutdown.exit. The 
actual serialization takes places in Shutdown.exit and serializes with signals 
too. But discussion of signals has no place here - I'm not even sure we 
document those ways of terminating the VM anywhere?

-

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


Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]

2022-07-07 Thread Kim Barrett
On Mon, 4 Jul 2022 12:09:46 GMT, David Holmes  wrote:

>>> Is "deadlock" accurate?
>> 
>> Yes.
>> 
>> In the context of the specification, "shutdown hook" means _application_ 
>> shutdown hook - as far as the specification is concerned, application 
>> shutdown hooks are the only kind of hooks. Right?
>> 
>> For example, the following will deadlock (when run with the changes in this 
>> PR):
>> 
>> 
>> public class TestHook {
>>   public static void main(String... arg) {
>> Thread hook = new Thread("my-hook") {
>>   @Override
>>   public void run() {
>> System.exit(1);
>>   }
>> };
>> Runtime.getRuntime().addShutdownHook(hook);
>> System.exit(0);
>>   }
>> }
>
> It is a general deadlock, not a monitor based deadlock: the thread that 
> called exit and holds the lock has to join() the hook thread. The hook thread 
> blocks on the lock held by the exiting thread. Neither thread can progress.

You folks are correct.  I hadn't noticed the ApplicationShutdownHooks thing.  
Sorry for the noise.

-

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


Re: [jdk19] RFR: 8289148: j.l.foreign.VaList::nextVarg call could throw IndexOutOfBoundsException or even crash the VM [v3]

2022-07-07 Thread Maurizio Cimadamore
On Tue, 5 Jul 2022 17:27:34 GMT, Jorn Vernee  wrote:

>> This patch changes all VaList implementations to throw 
>> `NoSuchElementException` when out of bounds reads occur on a VaList that is 
>> created using the Java builder API. The docs are updated accordingly.
>> 
>> For VaLists that are created from native addresses, we don't know their 
>> bounds, so we can not throw exceptions in that case.
>> 
>> Testing: `jdk_foreign` test suite on all platforms that implement VaList.
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   hmtl -> html

Looks good!

-

Marked as reviewed by mcimadamore (Reviewer).

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


Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]

2022-07-07 Thread Chris Hegarty
On Mon, 4 Jul 2022 12:19:27 GMT, David Holmes  wrote:

>> Ryan Ernst has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   better clarify multiple threads behavior
>
>> Let's say we've run shutdown so run all the hooks but not halted. Then 
>> someone calls exit(0). That seems to suggest the call will block 
>> indefinitely, which is neither desirable nor what was actually implemented.
> 
> If the hook threads do not halt then the exiting thread (which holds the 
> lock) blocks forever in the join(). Any other call to exit blocks trying to 
> acquire the lock. That has always been the way this works - if hook threads 
> don't terminate then the VM doesn't (unless someone calls halt() directly). 
> That is one of the things the window you suggested be closed, allowed - a 
> call to exit(non-zero) could force a call to halt().

@dholmes-ora @AlanBateman @kimbarrett The following 
[CSR](https://bugs.openjdk.org/browse/JDK-8289824) has been filed to cover the 
specification related changes in this PR - please review as appropriate.

-

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


Re: RFR: 8289711: Add container configuration data to mbeans

2022-07-07 Thread Jie Fu
On Tue, 5 Jul 2022 04:21:55 GMT, xpbob  wrote:

> Container configuration information is useful for troubleshooting 
> problems,Exposing information in MBeans is ideal for monitoring, jConsole, 
> and other scenarios.
> Results the following
> ![图片](https://user-images.githubusercontent.com/7837910/177248834-50beefe9-4db6-470c-8f15-df5a93892804.png)

src/java.base/share/classes/module-info.java line 233:

> 231: exports jdk.internal.platform to
> 232: jdk.management,
> 233: java.management,

Maybe, we'd better put `jdk.management,` before `jdk.management,`.

src/java.management/share/classes/java/lang/management/ContainerMXBean.java 
line 1:

> 1: package java.lang.management;

Shall we add a copyright header for this new file?

-

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


Re: RFR: 8288706: Unused parameter 'boolean newln' in method java.lang.VersionProps#print(boolean, boolean)

2022-07-07 Thread Roger Riggs
On Tue, 5 Jul 2022 18:38:57 GMT, Lance Andersen  wrote:

> Hi all,
> 
> This PR cleans up `VersionProps::print` removing the unused parameter `newln`.
> 
> Mach5 tiers1-3 are currently running.
> 
> Best,
> Lance

Marked as reviewed by rriggs (Reviewer).

-

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


Re: RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets

2022-07-07 Thread Ichiroh Takiguchi
On Wed, 6 Jul 2022 14:05:39 GMT, Ichiroh Takiguchi  
wrote:

> OpenJDK supports "Japanese EBCDIC - Katakana" and "Korean EBCDIC" SBCS and 
> DBCS Only charsets.
> |Charset|Mix|SBCS|DBCS|
> | -- | -- | -- | -- |
> | Japanese EBCDIC - Katakana | Cp930 | Cp290 | Cp300 |
> | Korean | Cp933 | Cp833 | Cp834 |
> 
> But OpenJDK does not supports some of "Japanese EBCDIC - English" / 
> "Simplified Chinese EBCDIC" / "Traditional Chinese EBCDIC" SBCS and DBCS Only 
> charsets.
> 
> I'd like to request Cp1027/Cp835/Cp836/Cp837 for consistency
> |Charset|Mix|SBCS|DBCS|
> | - | - | - | - |
> | Japanese EBCDIC - English | Cp939 | **Cp1027** | Cp300 |
> | Simplified Chinese EBCDIC | Cp935 | **Cp836** | **Cp837** |
> | Traditional Chinese EBCDIC | Cp937 | (*1) | **Cp835** | 
> 
> *1: Cp037 compatible

Discussions are available on :
[JDK-8289834](https://bugs.openjdk.org/browse/JDK-8289834): Add SBCS and DBCS 
Only EBCDIC charsets

-

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


Re: RFR: JDK-8289741 : Remove unused imports from DTDBuilder.java [v2]

2022-07-07 Thread ScientificWare
> This is tracked in JBS as
> 
> [JDK-8289741](https://bugs.openjdk.org/browse/JDK-8289741)
> 
>> **Remove unused imports from DTDBuilder.java**
> 
> Some imports are no more used.
> 
> - java.io.FileNotFoundException;
> - java.io.BufferedInputStream;
> - java.io.OutputStream;
> - java.util.BitSet;
> - java.util.StringTokenizer;
> - java.util.Properties;
> - java.util.zip.DeflaterOutputStream;
> - java.util.zip.Deflater;
> - java.net.URL;
> 
> https://github.com/scientificware/jdk/issues/4

ScientificWare 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 eight additional 
commits since the last revision:

 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder
 - Remove unused imports.
   
   Some imports are no more used.
   
   - java.io.FileNotFoundException;
   - java.io.BufferedInputStream;
   - java.io.OutputStream;
   - java.util.BitSet;
   - java.util.StringTokenizer;
   - java.util.Properties;
   - java.util.zip.DeflaterOutputStream;
   - java.util.zip.Deflater;
   - java.net.URL;

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9360/files
  - new: https://git.openjdk.org/jdk/pull/9360/files/2bf79718..6d097e77

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9360=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=9360=00-01

  Stats: 3364 lines in 138 files changed: 2399 ins; 468 del; 497 mod
  Patch: https://git.openjdk.org/jdk/pull/9360.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9360/head:pull/9360

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


Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v5]

2022-07-07 Thread Chris Hegarty
On Tue, 5 Jul 2022 18:43:26 GMT, Ryan Ernst  wrote:

>> This is a followup to simplify Shutdown.exit after the removal of
>> finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement
>> on the approach has been reached in this PR, a CSR will be filed to
>> propose the spec change to Runtime.exit.
>
> Ryan Ernst 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 five additional commits since 
> the last revision:
> 
>  - Merge branch 'master' into shutdown_cleanup
>  - iter text
>  - iterate on wording
>  - better clarify multiple threads behavior
>  - 8288984: Simplification in Shutdown.exit
>
>This is a followup to simplify Shutdown.exit after the removal of
>finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement
>on the approach has been reached in this PR, a CSR will be filed to
>propose the spec change to Runtime.exit.

Marked as reviewed by chegar (Reviewer).

-

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


Re: RFR: 8288838: jpackage: file association additional arguments [v4]

2022-07-07 Thread Alexey Semenyuk
On Mon, 4 Jul 2022 23:38:08 GMT, Alex Kasko  wrote:

>> jpackage implementation of file association on Windows currently passes a 
>> selected filename as an only argument to associated executable.
>> 
>> It is proposed to introduce additional option in file association property 
>> file to allow optionally support additional arguments using `%*` batch 
>> wildcard.
>> 
>> Note, current implementation, while fully functional, is only a **DRAFT** 
>> one, it is not ready for integration in this form. I would appreciate any 
>> guidance on the following points:
>> 
>>  - option naming inside a properties file, currently `pass-all-args` is used
>>  - option naming in a bundler parameter implementation, it is not clear if 
>> it should introduce a new group of "file association windows specific 
>> options" next to the existing "file association mac specific options" group
>>  - test organization to cover the new option: currently it is included 
>> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) 
>> `includeDescription` parameter; it is not clear whether it should be done in 
>> a separate test and whether to include runs for every parameter combination
>>  - test run implementation: currently arguments are checked when a file with 
>> associated extension is invoked from command line; it is not clear whether 
>> it would be more appropriate instead to create a desktop shortcut with the 
>> same command as a target and to invoke it with `java.awt.Desktop`
>> 
>> Also please note, that full install/uninstall run is currently enabled in 
>> `FileAssociationTest`, it is intended to be used only in a draft code during 
>> the development and to be removed (to use the same "install or unpack" logic 
>> as other tests) in a final version.
>> 
>> Testing:
>>  
>> - [x] test to cover new logic is included
>> - [x] ran jtreg:jdk/tools/jpackage with no new failures
>
> Alex Kasko has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains three commits:
> 
>  - enable passing args, add test coverage
>  - Added input params validation
>  - Proposed test changes to make FA testing of jpackage more flexible

Marked as reviewed by asemenyuk (Reviewer).

I confirm the patch works and there are no regressions.

SQE uses only the part of jpackage jtreg tests that create test packages, so 
there is no point in customizing tests with reading args from the file system. 
I think I have enough information to help SQE update test spec and cover 
Unicode args.

-

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


Integrated: Merge jdk19

2022-07-07 Thread Jesper Wilhelmsson
On Wed, 6 Jul 2022 13:01:02 GMT, Jesper Wilhelmsson  
wrote:

> Forwardport JDK 19 -> JDK 20

This pull request has now been integrated.

Changeset: 2a6ec88c
Author:Jesper Wilhelmsson 
URL:   
https://git.openjdk.org/jdk/commit/2a6ec88cd09adec43df3da1b22653271517b14a8
Stats: 888 lines in 24 files changed: 705 ins; 64 del; 119 mod

Merge

-

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]

2022-07-07 Thread Stuart Marks
On Wed, 6 Jul 2022 23:03:42 GMT, Stuart Marks  wrote:

>> Simple javadoc fix of an editorial nature.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflow and adjust whitespace per Alan's & Iris' comments.

Reflowed and adjusted whitespace.

Also, Pavel wrote:
> Also, consider closing 8255426 as a duplicate.

Ha! Thanks, I had forgotten about that one. Easier to file a new bug than to 
dig out an old one. Oh well, closed as duplicate.

-

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


Re: RFR: 8288723: Avoid redundant ConcurrentHashMap.get call in java.time [v2]

2022-07-07 Thread Roger Riggs
On Tue, 5 Jul 2022 21:27:16 GMT, Attila Szegedi  wrote:

>> Can there be some JMH tests to confirm the performance?
>> The value domain of the keys is pretty limited (7 * 7) max; and I'm not sure 
>> that the combination of creating a new record and the hashcode and equals 
>> methods would be faster than string concat of a constant and a single digit 
>> integer.
>
> @RogerRiggs in my view, it's not about performance per se… I'll sometimes 
> help along these kinds of small fix PRs (that are more likely than not driven 
> by IntelliJ code refactoring suggestions) if I think they lead to more 
> idiomatic code in the JDK with no drawbacks. I think there's value in 
> periodically revisiting existing code and improve it to use new language and 
> library construct that have become available since – people study JDK source 
> code, and I think it's important they see as good examples in there as 
> possible. I do like seeing an old example of hand-rolled compute-if-absent 
> being replaced by an actual `computeIfAbsent` call. Similarly, I think using 
> records as composite keys is in general a better practice over simulating a 
> composite key by creating a string representation of it. 
> 
> Yeah, it'll load additional code at runtime for that record's class, and yes, 
> I'm aware records' `hashCode`/`equals`/`toString` are implemented by 
> delegating to factory methods that heavily use method handle combinators. If 
> records are meant as lightweight value-semantics composites, it should be 
> okay to use them like this and if there are performance concerns with their 
> use, then those concerns should be addressed by improving their performance. 
> OTOH, MH combinators installed into constant call sites are well optimized by 
> HotSpot these days… anyhow, a discussion for another day.
> 
> In this specific instance, the value domain of the keys is limited, but the 
> number of method calls to `WeekFields.of` isn't. It could be benchmarked but 
> if there's concerns, it's also okay to de-scope the record-as-key from this 
> PR. I don't have strong feelings either way.

The comment was about WeekFields.of(), I misplaced the comment.

@szegedi All good points about modernizing code...
One of the reasons to ask about specific performance data is to validate the 
general performance impact of using lambdas. In the case of WeekFields.of(), 
the lambda is passed on every call to `computeIfAbsent` even if the key is 
present and the lambda won't be used. `WeekFields` is way-way down the long 
tail of frequency of use and I expect that 99% of calls are for the same one or 
two combinations.

-

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


Integrated: 8289260: BigDecimal movePointLeft() and movePointRight() do not follow their API spec

2022-07-07 Thread Raffaello Giulietti
On Tue, 28 Jun 2022 10:09:36 GMT, Raffaello Giulietti  wrote:

> `BigDecimal.morePoint[Left|Right]()` should return the target `this` when the 
> argument is 0 _and_ the scale is non-negative.

This pull request has now been integrated.

Changeset: 35387d5c
Author:Raffaello Giulietti 
Committer: Joe Darcy 
URL:   
https://git.openjdk.org/jdk/commit/35387d5cb6aa9e59d62b8e1b137b53ec88521310
Stats: 108 lines in 2 files changed: 106 ins; 0 del; 2 mod

8289260: BigDecimal movePointLeft() and movePointRight() do not follow their 
API spec

Reviewed-by: darcy

-

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


Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses

2022-07-07 Thread Pavel Rappo
On Wed, 6 Jul 2022 00:32:00 GMT, Stuart Marks  wrote:

> Simple javadoc fix of an editorial nature.

Thanks for doing this, Stuart. Not only does this PR remove duplication from 
the `Map` documentation, but it also ensures that when JDK-6509045 is 
integrated, that duplication won't spread to `ConcurrentMap`. A bonus of this 
fix is that the irrelevant mention of "keys" is gone from the description of 
`NullPointerException`.

Also, consider closing 8255426 as a duplicate.

-

Marked as reviewed by prappo (Reviewer).

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


Re: RFR: 8289768: Clean up unused code [v2]

2022-07-07 Thread Lance Andersen
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński  wrote:

>> This patch removes many unused variables and one unused label reported by 
>> the compilers when relevant warnings are enabled. 
>> 
>> The unused code was found by compiling after removing `unused` from the list 
>> of disabled warnings for 
>> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190)
>>  and 
>> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203),
>>  and enabling 
>> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170)
>>  MSVC warning.
>> 
>> I only removed variables that were uninitialized or initialized without side 
>> effects. I verified that the removed variables were not used in any 
>> `#ifdef`'d code. I checked that the changed code still compiles on Windows, 
>> Linux and Mac, both in release and debug versions.
>
> Daniel Jeliński 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 four additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin' into unused-variables
>  - Remove unused variables (windows)
>  - Remove unused variables (macos)
>  - Remove unused variables (linux)

zlib looks fine given it is openjdk and not an upstream needed change

-

Marked as reviewed by lancea (Reviewer).

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


Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG

2022-07-07 Thread Alexander Matveev
On Wed, 29 Jun 2022 03:03:15 GMT, Alexander Matveev  
wrote:

> Fixed 3 issues which made signature invalid:
> - We should not remove .jpackage.xml from signed app image when creating DMG 
> or PKG otherwise it invalidates signature.
> - .package should be created when app image is generated, so this file can be 
> signed.
> - Copying predefine app image for DMG and PKG should not follow symbolic 
> links, otherwise several files from runtime (COPYRIGHT and LICENSE) will be 
> copied instead of symbolic links being created, since it invalidates 
> signature as well.
> 
> Added additional test to validate signature when DMG or PKG is generated from 
> predefined app image.

8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
 - Fixed as suggested by Alexey. If app image already signed we will not add 
`.package` file.

-

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


Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]

2022-07-07 Thread Jorn Vernee
On Wed, 6 Jul 2022 17:05:51 GMT, Jorn Vernee  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert implicit vs. heap session changes
>
> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template
>  line 131:
> 
>> 129: AbstractMemorySegmentImpl bb = checkAddress(obb, base, 
>> handle.length, true);
>> 130: #if[floatingPoint]
>> 131: $rawType$ rawValue = 
>> SCOPED_MEMORY_ACCESS.get$RawType$Unaligned(bb.session(),
> 
> For instance, it's not clear to me why `baseSession()` is not called here.

It seems that, if `get$RawType$Unaligned` calls `checkValidStateRaw()` this 
will end up throwing an exception

-

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


Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]

2022-07-07 Thread Jorn Vernee
On Fri, 17 Jun 2022 18:39:03 GMT, Maurizio Cimadamore  
wrote:

>> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Revert implicit vs. heap session changes

Am I understanding correctly that, before calling `checkValidState` we also 
need to always call `baseSession()`? `checkValidState()` just calls 
`checkValidStateRaw` in a try/catch, and for non-closeable views this always 
throws.

There seem to be many cases where `baseSession()` is not called...

-

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


Re: [jdk19] RFR: 8289601: SegmentAllocator::allocateUtf8String(String str) should be clarified for strings containing \0

2022-07-07 Thread Jorn Vernee
On Mon, 4 Jul 2022 13:01:34 GMT, Jorn Vernee  wrote:

> This PR updates the spec and implementation to throw an 
> `IllegalArgumentException` when an attempt is made to convert a Java string 
> containing null characters to a C string.
> 
> Testing: local run of the `jdk_foreign` test suite.

I've tweaked the Javadoc. I realized it should also be tweaked for 
`MemorySegment::setUtf8String`.

-

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


Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]

2022-07-07 Thread Jorn Vernee
On Fri, 17 Jun 2022 18:39:03 GMT, Maurizio Cimadamore  
wrote:

>> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Revert implicit vs. heap session changes

src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template
 line 131:

> 129: AbstractMemorySegmentImpl bb = checkAddress(obb, base, 
> handle.length, true);
> 130: #if[floatingPoint]
> 131: $rawType$ rawValue = 
> SCOPED_MEMORY_ACCESS.get$RawType$Unaligned(bb.session(),

For instance, it's not clear to me why `baseSession()` is not called here.

src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/macos/MacOsAArch64VaList.java
 line 172:

> 170: 
> 171: public Builder(MemorySession session) {
> 172: ((MemorySessionImpl)session).checkValidState();

Or here, if the memory session is a non-closeable view.

-

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


Re: RFR: 8279283 - BufferedInputStream should override transferTo [v5]

2022-07-07 Thread Markus KARG
On Mon, 27 Dec 2021 13:43:12 GMT, Markus KARG  wrote:

>> Implementation of JDK-8279283
>
> Markus KARG has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixed missing BufferedInputStream

Please keep this PR open; I will continune work on it soon.

-

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


RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets

2022-07-07 Thread Ichiroh Takiguchi
OpenJDK supports "Japanese EBCDIC - Katakana" and "Korean EBCDIC" SBCS and DBCS 
Only charsets.
|Charset|Mix|SBCS|DBCS|
| -- | -- | -- | -- |
| Japanese EBCDIC - Katakana | Cp930 | Cp290 | Cp300 |
| Korean | Cp933 | Cp833 | Cp834 |

But OpenJDK does not supports some of "Japanese EBCDIC - English" / "Simplified 
Chinese EBCDIC" / "Traditional Chinese EBCDIC" SBCS and DBCS Only charsets.

I'd like to request Cp1027/Cp835/Cp836/Cp837 for consistency
|Charset|Mix|SBCS|DBCS|
| - | - | - | - |
| Japanese EBCDIC - English | Cp939 | **Cp1027** | Cp300 |
| Simplified Chinese EBCDIC | Cp935 | **Cp836** | **Cp837** |
| Traditional Chinese EBCDIC | Cp937 | (*1) | **Cp835** | 

*1: Cp037 compatible

-

Commit messages:
 - 8289834: Missing SBCS and DBCS Only EBCDIC charsets

Changes: https://git.openjdk.org/jdk/pull/9399/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=9399=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289834
  Stats: 369 lines in 6 files changed: 367 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/9399.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9399/head:pull/9399

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


Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v14]

2022-07-07 Thread Сергей Цыпанов
On Thu, 30 Jun 2022 16:41:36 GMT, iaroslavski  wrote:

>> Sorting:
>> 
>> - adopt radix sort for sequential and parallel sorts on 
>> int/long/float/double arrays (almost random and length > 6K)
>> - fix tryMergeRuns() to better handle case when the last run is a single 
>> element
>> - minor javadoc and comment changes
>> 
>> Testing:
>> - add new data inputs in tests for sorting
>> - add min/max/infinity values to float/double testing
>> - add tests for radix sort
>
> iaroslavski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
>   
>   * Fix @since version

I mean adding benchmarks into `micro.org.openjdk.bench`

-

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


Re: RFR: 8289768: Clean up unused code [v2]

2022-07-07 Thread Daniel Jeliński
> This patch removes many unused variables and one unused label reported by the 
> compilers when relevant warnings are enabled. 
> 
> The unused code was found by compiling after removing `unused` from the list 
> of disabled warnings for 
> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190)
>  and 
> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203),
>  and enabling 
> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170)
>  MSVC warning.
> 
> I only removed variables that were uninitialized or initialized without side 
> effects. I verified that the removed variables were not used in any 
> `#ifdef`'d code. I checked that the changed code still compiles on Windows, 
> Linux and Mac, both in release and debug versions.

Daniel Jeliński 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 four additional 
commits since the last revision:

 - Merge remote-tracking branch 'origin' into unused-variables
 - Remove unused variables (windows)
 - Remove unused variables (macos)
 - Remove unused variables (linux)

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9383/files
  - new: https://git.openjdk.org/jdk/pull/9383/files/915680c0..b4cd5374

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9383=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=9383=00-01

  Stats: 491 lines in 22 files changed: 458 ins; 5 del; 28 mod
  Patch: https://git.openjdk.org/jdk/pull/9383.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9383/head:pull/9383

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


Re: RFR: 8289768: Clean up unused code [v2]

2022-07-07 Thread Weijun Wang
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński  wrote:

>> This patch removes many unused variables and one unused label reported by 
>> the compilers when relevant warnings are enabled. 
>> 
>> The unused code was found by compiling after removing `unused` from the list 
>> of disabled warnings for 
>> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190)
>>  and 
>> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203),
>>  and enabling 
>> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170)
>>  MSVC warning.
>> 
>> I only removed variables that were uninitialized or initialized without side 
>> effects. I verified that the removed variables were not used in any 
>> `#ifdef`'d code. I checked that the changed code still compiles on Windows, 
>> Linux and Mac, both in release and debug versions.
>
> Daniel Jeliński 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 four additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin' into unused-variables
>  - Remove unused variables (windows)
>  - Remove unused variables (macos)
>  - Remove unused variables (linux)

Changes in security (`java.security.jgss`, `jdk.crypto.cryptoki`, 
`jdk.crypto.mscapi`) look fine to me.

-

Marked as reviewed by weijun (Reviewer).

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