Re: RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v6]

2023-08-11 Thread Claes Redestad 
On Wed, 9 Aug 2023 22:54:28 GMT, Pavel Rappo  wrote:

>> Please review this PR to use modern APIs and language features to simplify 
>> equals, hashCode, and compareTo for BigInteger. If you have any performance 
>> concerns, please raise them.
>> 
>> This PR is cherry-picked from a bigger, not-yet-published PR, to test the 
>> waters. That latter PR will be published soon.
>
> Pavel Rappo 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 13 additional 
> commits since the last revision:
> 
>  - Fix bugs in Shared.createSingle
>  - Merge branch 'master' into 8310813
>  - Group params coarser (suggested by @cl4es)
>
>- Splits 20 params into 3 groups: (S)mall, (M)edium and (L)arge.
>  Every testXYZ method invokes M operations, where M is the maximum
>  number of elements in a group. Shorter groups are cyclically padded.
>- Uses the org.openjdk.jmh.infra.Blackhole API and increases
>  benchmark time.
>- Fixes a bug in Shared that precluded 0 from being in a pair.
>  - Use better overloads (suggested by @cl4es)
>
>- Uses simpler, more suitable overloads for the subrange
>  starting from 0
>  - Improve benchmarks
>  - Merge branch 'master' into 8310813
>  - Restore hash code values
>
>BigInteger is old and ubiquitous enough so that there might be
>inadvertent dependencies on its hash code.
>
>This commit also includes a test, to make sure hash code is
>unchanged.
>  - Merge branch 'master' into 8310813
>  - Add a benchmark
>  - Merge branch 'master' into 8310813
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/983eb997...6fa3f694

> Here's a status update for this PR. I'm currently benchmarking baseline 
> against this PR and this PR plus changes in #15197. It's a 3-way benchmark, 
> so to speak. Its purpose is to see whether performance degradation brought by 
> this PR to `equals` and `compareTo` can be sufficiently offset by the 
> improved `ArraysSupport.mismatch` mechanics.

Sadly #15197 doesn't pan out as well as I'd hoped: the win on some array sizes 
is cancelled out by losses on others. I'll be shifting the focus of that PR 
over to simplifying in ways that will be performance neutral and enhancing the 
targeted `ArraysMismatch` microbenchmarks so that they work on tiny array sizes.

-

PR Comment: https://git.openjdk.org/jdk/pull/14630#issuecomment-1674563442

Re: RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v6]

2023-08-11 Thread Claes Redestad 
On Thu, 10 Aug 2023 11:38:08 GMT, Pavel Rappo  wrote:

>> Pavel Rappo 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 13 additional 
>> commits since the last revision:
>> 
>>  - Fix bugs in Shared.createSingle
>>  - Merge branch 'master' into 8310813
>>  - Group params coarser (suggested by @cl4es)
>>
>>- Splits 20 params into 3 groups: (S)mall, (M)edium and (L)arge.
>>  Every testXYZ method invokes M operations, where M is the maximum
>>  number of elements in a group. Shorter groups are cyclically padded.
>>- Uses the org.openjdk.jmh.infra.Blackhole API and increases
>>  benchmark time.
>>- Fixes a bug in Shared that precluded 0 from being in a pair.
>>  - Use better overloads (suggested by @cl4es)
>>
>>- Uses simpler, more suitable overloads for the subrange
>>  starting from 0
>>  - Improve benchmarks
>>  - Merge branch 'master' into 8310813
>>  - Restore hash code values
>>
>>BigInteger is old and ubiquitous enough so that there might be
>>inadvertent dependencies on its hash code.
>>
>>This commit also includes a test, to make sure hash code is
>>unchanged.
>>  - Merge branch 'master' into 8310813
>>  - Add a benchmark
>>  - Merge branch 'master' into 8310813
>>  - ... and 3 more: https://git.openjdk.org/jdk/compare/3619ef21...6fa3f694
>
> Here's a status update for this PR. I'm currently benchmarking baseline 
> against this PR and this PR plus changes in 
> https://github.com/openjdk/jdk/pull/15197. It's a 3-way benchmark, so to 
> speak. Its purpose is to see whether performance degradation brought by this 
> PR to `equals` and `compareTo` can be sufficiently offset by the improved 
> `ArraysSupport.mismatch` mechanics.

Some context: On the `equals` and `compareTo` microbenchmarks @pavelrappo is 
adding in this PR there's a tiny regression from using `ArraysSupport.mismatch` 
when the `value` array has just a single element. Since such small 
`BigInteger`s appears to be exceedingly common (huh) it's been deemed hard to 
brush aside. Still this regression comes from the code having some extra 
branch, and is less than 2ns/op. All the while the speedup on huge 
`BigInteger`s is substantial. 

I think it's reasonable to move forward with this patch and accept the tiny 
regression on `equals/hashCode` with a magnitude of 1 (like we've done in other 
places such as `String.startsWith`) and separately continue investigating if 
`ArraysSupport.mismatch` can be improved to remove this deficiency.

-

PR Comment: https://git.openjdk.org/jdk/pull/14630#issuecomment-1674582483

Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v4]

2023-08-11 Thread Jorn Vernee 
> This patch contains the implementation of the foreign linker & memory API JEP 
> for Java 22. The initial patch is composed of commits brought over directly 
> from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). 
> The main changes found in this patch come from the following PRs:
> 
> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous 
> iterations supported converting Java strings to and from native strings in 
> the UTF-8 encoding, we've extended the supported encoding to all the 
> encodings found in the `java.nio.charset.StandardCharsets` class.
> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the 
> `MemoryLayout::sequenceLayout` factory method which inferred the size of the 
> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A 
> client is now required to explicitly specify the sequence size.
> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: 
> `Linker::canonicalLayouts`, which exposes a map containing the 
> platform-specific mappings of common C type names to memory layouts.
> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access 
> varhandles, as well as byte offset and slice handles derived from memory 
> layouts, now feature an additional 'base offset' coordinate that is added to 
> the offset computed by the handle. This allows composing these handles with 
> other offset computation strategies that may not be based on the same memory 
> layout. This addresses use-cases where clients are working with 'dynamic' 
> layouts, whose size might not be known statically, such as variable length 
> arrays, or variable size matrices.
> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now 
> redundant API. Clients can simply use the difference between the base address 
> of two memory segments.
> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of 
> `SegmentAllocator::allocateArray`, by renaming methods that both allocate + 
> initialize memory segments to `allocateFrom`. (see the original PR for the 
> problematic case)
> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the 
> documentation for variadic functions.
> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to 
> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking 
> linux-x86 as a test bed.
> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API 
> required. The `Linker::nativeLinker` method is not longer allowed to throw an 
> `UnsupportedOperationException` on ...

Jorn Vernee has updated the pull request incrementally with one additional 
commit since the last revision:

  remove spurious imports

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15103/files
  - new: https://git.openjdk.org/jdk/pull/15103/files/147e79d3..141096b8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=15103&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15103&range=02-03

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

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

Re: RFR: 8314120: Add tests for FileDescriptor.sync

2023-08-11 Thread Alan Bateman 
On Thu, 10 Aug 2023 15:45:12 GMT, Aleksey Shipilev  wrote:

> When backporting [JDK-8312127](https://bugs.openjdk.org/browse/JDK-8312127), 
> I realized there are no targeted tests for `FileDescriptor.sync` that can be 
> used to qualify the changes in that area. 
> 
> Additionally, we use `FD.sync` for durability in Java databases, and we want 
> to make sure at least some smoke tests are available in OpenJDK. Asserting 
> durability would be hard, but let's at least test the Java code does not 
> throw unexpected exceptions and native code does not crash the VM.
> 
> The benchmark will show, among other things, that the recent change to 
> `FileDescriptor.sync` does not affect the performance much, compared to the 
> cost of the `fsync` itself. It deliberately targets tmpfs to provide the 
> lowest actual FS overhead.
> 
> 
> BenchmarkMode  CntScore   Error  Units
> 
> # Before JDK-8312127
> FileDescriptorSync.sync  avgt   15  351,688 ? 2,477  ns/op
> 
> # After JDK-8312127
> FileDescriptorSync.sync  avgt   15  353,331 ? 2,116  ns/op
> 
> 
> The new regression test completes in <0.5s on my Mac.

It's a historical issue that the tests for this are elsewhere so it's good to 
add some sanity test for this legacy method.

test/jdk/java/io/FileDescriptor/Sync.java line 36:

> 34:  * @requires vm.continuations
> 35:  * @library /test/lib
> 36:  * @run main/othervm -XX:+UnlockExperimentalVMOptions 
> -XX:-VMContinuations Sync

Running it with -XX:-VMContinuations is probably overkill here as it will be 
the same as the default case.

test/jdk/java/io/FileDescriptor/Sync.java line 83:

> 81: 
> 82: File tmpFile = File.createTempFile("FileDescriptorSync2", "tmp");
> 83: testWith(tmpFile);

This will leave the file on the tmp file system. A try-finally to delete it 
might be best as we've had issues with temp file accumulating in long test runs.

-

PR Review: https://git.openjdk.org/jdk/pull/15231#pullrequestreview-1573793329
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291415629
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291417472

Re: RFR: 8314120: Add tests for FileDescriptor.sync [v2]

2023-08-11 Thread Aleksey Shipilev 
> When backporting [JDK-8312127](https://bugs.openjdk.org/browse/JDK-8312127), 
> I realized there are no targeted tests for `FileDescriptor.sync` that can be 
> used to qualify the changes in that area. 
> 
> Additionally, we use `FD.sync` for durability in Java databases, and we want 
> to make sure at least some smoke tests are available in OpenJDK. Asserting 
> durability would be hard, but let's at least test the Java code does not 
> throw unexpected exceptions and native code does not crash the VM.
> 
> The benchmark will show, among other things, that the recent change to 
> `FileDescriptor.sync` does not affect the performance much, compared to the 
> cost of the `fsync` itself. It deliberately targets tmpfs to provide the 
> lowest actual FS overhead.
> 
> 
> BenchmarkMode  CntScore   Error  Units
> 
> # Before JDK-8312127
> FileDescriptorSync.sync  avgt   15  351,688 ? 2,477  ns/op
> 
> # After JDK-8312127
> FileDescriptorSync.sync  avgt   15  353,331 ? 2,116  ns/op
> 
> 
> The new regression test completes in <0.5s on my Mac.

Aleksey Shipilev has updated the pull request incrementally with one additional 
commit since the last revision:

  Review comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15231/files
  - new: https://git.openjdk.org/jdk/pull/15231/files/19f8c6fb..e66c244a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=15231&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15231&range=00-01

  Stats: 32 lines in 1 file changed: 18 ins; 8 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/15231.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15231/head:pull/15231

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

Re: RFR: 8314120: Add tests for FileDescriptor.sync [v2]

2023-08-11 Thread Aleksey Shipilev 
On Fri, 11 Aug 2023 14:32:29 GMT, Alan Bateman  wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Review comments
>
> test/jdk/java/io/FileDescriptor/Sync.java line 36:
> 
>> 34:  * @requires vm.continuations
>> 35:  * @library /test/lib
>> 36:  * @run main/othervm -XX:+UnlockExperimentalVMOptions 
>> -XX:-VMContinuations Sync
> 
> Running it with -XX:-VMContinuations is probably overkill here as it will be 
> the same as the default case.

Agreed, removed.

> test/jdk/java/io/FileDescriptor/Sync.java line 83:
> 
>> 81: 
>> 82: File tmpFile = File.createTempFile("FileDescriptorSync2", "tmp");
>> 83: testWith(tmpFile);
> 
> This will leave the file on the tmp file system. A try-finally to delete it 
> might be best as we've had issues with temp file accumulating in long test 
> runs.

Agreed. I think `try-with-resources` wrapper is cleaner here. See new commit.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291452045
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291457992

Re: RFR: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-11 Thread Alexey Semenyuk 
On Thu, 10 Aug 2023 22:58:18 GMT, Alexander Matveev  
wrote:

> - This is regression from 
> [JDK-8298488](https://bugs.openjdk.org/browse/JDK-8298488).
> - Since JDK-8298488 unsigned app bundles are ad-hoc signed and `codesign` 
> will report that app bundle is signed and thus our tests failed.
> - Fixed tests by checking that all app bundles are signed and by checking how 
> they signed ad-hoc vs actual certificate.
> - Unsigned post process image will be ad-hoc re-sign when generating DMG or 
> PKG, since we adding `.package` file which makes ad-hoc signature invalid. 
> This is similar to [JDK-8293462](https://bugs.openjdk.org/browse/JDK-8293462).

Marked as reviewed by asemenyuk (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15235#pullrequestreview-1573979900

Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v3]

2023-08-11 Thread Jorn Vernee 
On Thu, 10 Aug 2023 23:31:57 GMT, Chen Liang  wrote:

> Just curious, what's the rationale for finalizing the API when there are 
> significant changes from the last preview?

A preview API is finalized when it is ready. The preview process, as outlined 
by [JEP 12](https://bugs.openjdk.org/browse/JDK-8195734), does not place a 
mandate on the amount of changes that a JEP that finalizes a preview API should 
or should not contain. It only requires that the changes since the last preview 
iteration are noted (which we have done). Though, the amount of changes can be 
used to inform the decision to finalize. We feel that the FFM API is ready for 
finalization, and does not require another round of preview.

In this case in particular: previous iterations contained significant changes 
to the API, including re-shuffling of some of the core APIs. (See e.g. 
https://github.com/openjdk/jdk/pull/13079#issuecomment-1476648707) In contrast 
this JEP contains mostly superficial changes to the API, that are not likely to 
impact how a client would write a program using the FFM API.

-

PR Comment: https://git.openjdk.org/jdk/pull/15103#issuecomment-1675057357

Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v4]

2023-08-11 Thread Uwe Schindler 
On Fri, 11 Aug 2023 12:37:59 GMT, Jorn Vernee  wrote:

>> This patch contains the implementation of the foreign linker & memory API 
>> JEP for Java 22. The initial patch is composed of commits brought over 
>> directly from the [panama-foreign 
>> repo](https://github.com/openjdk/panama-foreign). The main changes found in 
>> this patch come from the following PRs:
>> 
>> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous 
>> iterations supported converting Java strings to and from native strings in 
>> the UTF-8 encoding, we've extended the supported encoding to all the 
>> encodings found in the `java.nio.charset.StandardCharsets` class.
>> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the 
>> `MemoryLayout::sequenceLayout` factory method which inferred the size of the 
>> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A 
>> client is now required to explicitly specify the sequence size.
>> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: 
>> `Linker::canonicalLayouts`, which exposes a map containing the 
>> platform-specific mappings of common C type names to memory layouts.
>> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access 
>> varhandles, as well as byte offset and slice handles derived from memory 
>> layouts, now feature an additional 'base offset' coordinate that is added to 
>> the offset computed by the handle. This allows composing these handles with 
>> other offset computation strategies that may not be based on the same memory 
>> layout. This addresses use-cases where clients are working with 'dynamic' 
>> layouts, whose size might not be known statically, such as variable length 
>> arrays, or variable size matrices.
>> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now 
>> redundant API. Clients can simply use the difference between the base 
>> address of two memory segments.
>> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of 
>> `SegmentAllocator::allocateArray`, by renaming methods that both allocate + 
>> initialize memory segments to `allocateFrom`. (see the original PR for the 
>> problematic case)
>> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the 
>> documentation for variadic functions.
>> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to 
>> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking 
>> linux-x86 as a test bed.
>> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API 
>> required. The `Linker::nativeLinker` method is not longer allowed to throw 
>> an `UnsupportedO...
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove spurious imports

> > Just curious, what's the rationale for finalizing the API when there are 
> > significant changes from the last preview?
> 
> A preview API is finalized when it is ready. The preview process, as outlined 
> by [JEP 12](https://bugs.openjdk.org/browse/JDK-8195734), does not place a 
> mandate on the amount of changes that a JEP that finalizes a preview API 
> should or should not contain. It only requires that the changes since the 
> last preview iteration are noted (which we have done). Though, the amount of 
> changes can be used to inform the decision to finalize. We feel that the FFM 
> API is ready for finalization, and does not require another round of preview.
> 
> In this case in particular: previous iterations contained significant changes 
> to the API, including re-shuffling of some of the core APIs. (See e.g. 
> [#13079 
> (comment)](https://github.com/openjdk/jdk/pull/13079#issuecomment-1476648707))
>  In contrast this JEP contains mostly superficial changes to the API, that 
> are not likely to impact how a client would write a program using the FFM API.

In addition if somebody wrote code against the preview API, heshe needs to 
update it anyways because the class files are marked by preview flag. So all is 
fine. We just make API ready to use for everybody and therefor all early 
adopters need to adapt. t won't affect anybox else.

To me it would be strange if code goes out of preview without changes, because 
if there are no changes why was it in preview then in the last JDK feature 
version?

-

PR Comment: https://git.openjdk.org/jdk/pull/15103#issuecomment-1675101701

RFR: 8314169: Combine related RoundingMode logic in j.text.DigitList

2023-08-11 Thread Justin Lu 
Please review this PR which is a broad clean up of the DigitList class (used by 
Format classes in j.text).

This PR is intended to be a portion of a bigger change (split up to make 
reviewing easier). 

The main change combines related Rounding Mode logic in `shouldRoundUp()` - 
(_CEILING/FLOOR_, _HALF_UP/DOWN/EVEN_)

Other changes include
- Certain for loops can be replaced with cleaner syntax (E.g. for(;;), empty 
for loops)
- Introduce overloaded `round(int)` - For use by Integer representations of 
DigitList
- Introduce `non0AfterIndex(int)` - To reduce code duplication

-

Commit messages:
 - Init

Changes: https://git.openjdk.org/jdk/pull/15252/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15252&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8314169
  Stats: 166 lines in 1 file changed: 59 ins; 68 del; 39 mod
  Patch: https://git.openjdk.org/jdk/pull/15252.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15252/head:pull/15252

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

Re: RFR: 8313657 : com.sun.jndi.ldap.Connection.cleanup does not close connections on SocketTimeoutErrors [v5]

2023-08-11 Thread Aleksei Efimov 
On Wed, 9 Aug 2023 12:52:35 GMT, Weibing Xiao  wrote:

>> com.sun.jndi.ldap.Connection::leanup does not close the underlying socket if 
>> the is an IOException generation when the output stream was flushing the 
>> buffer.
>> 
>> Please refer to the bug https://bugs.openjdk.org/browse/JDK-8313657.
>
> Weibing Xiao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   class access modifier

test/jdk/com/sun/jndi/ldap/SocketCloseTest.java line 116:

> 114: 
> 115: private static class LdapInputStream extends InputStream {
> 116: private LdapOutputStream los;

The `LdapOutputStream` reference is not used by `LdapInputStream`, therefore 
the `los` field can be removed:

private static class LdapInputStream extends InputStream {
-private LdapOutputStream los;
 private ByteArrayInputStream bos;
 
-public LdapInputStream(LdapOutputStream los) {
-this.los = los;
+public LdapInputStream() {
 }
 
 @Override
@@ -144,7 +142,7 @@ public class SocketCloseTest {
 private static class CustomSocket extends Socket {
 private int closeMethodCalled = 0;
 private LdapOutputStream output = new LdapOutputStream();
-private LdapInputStream input = new LdapInputStream(output);
+private LdapInputStream input = new LdapInputStream();
 
 public void connect(SocketAddress address, int timeout) {
 }

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15143#discussion_r1291679154

Re: [External] : Re: RFR: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-11 Thread Alexander Matveev 
Hi Michael,

On Aug 10, 2023, at 10:48 PM, Michael Hall 
mailto:mik3h...@gmail.com>> wrote:



I assume done with jpackage by indicating something like —mac-sign only? If 
wrong feel free to correct.

No, it is always done if —mac-sign is NOT specified and we doing ad-hoc signing 
on app bundle only. PKG will not be ad-hoc signed.
If —mac-sign is provided we will use certificate provided with —mac-sign to 
sign app image and PKG as before.

Thanks,
Alexander

I always do dmg or app-image for quick testing. I haven’t done any pkg yet.

So if I understand correctly now for app bundle types - dmg or app-image, some 
signing is always done. Certificate if signing is indicated and it is provided 
or adhoc if it isn’t indicated as a default. I didn’t know this.

Yes, but ad-hoc signing was introduced back in JDK 20 with 
https://bugs.openjdk.org/browse/JDK-8298488 for macOS x64 and in JDK 19 with 
https://bugs.openjdk.org/browse/JDK-8277493 for macOS aarch64. We do ad-hoc 
signing for app images when generating PKG as well, so it is for all targets 
and not only for app-image or dmg.

Thanks,
Alexander

Re: [External] : Re: RFR: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-11 Thread Michael Hall 


> On Aug 11, 2023, at 3:55 PM, Alexander Matveev  
> wrote:
> 
> Hi Michael,
> 
>> On Aug 10, 2023, at 10:48 PM, Michael Hall > > wrote:
>> 
>> 
 
 I assume done with jpackage by indicating something like —mac-sign only? 
 If wrong feel free to correct.
>>> 
>>> No, it is always done if —mac-sign is NOT specified and we doing ad-hoc 
>>> signing on app bundle only. PKG will not be ad-hoc signed.
>>> If —mac-sign is provided we will use certificate provided with —mac-sign to 
>>> sign app image and PKG as before.
>>> 
>>> Thanks,
>>> Alexander
>> 
>> I always do dmg or app-image for quick testing. I haven’t done any pkg yet. 
>> 
>> So if I understand correctly now for app bundle types - dmg or app-image, 
>> some signing is always done. Certificate if signing is indicated and it is 
>> provided or adhoc if it isn’t indicated as a default. I didn’t know this.
> 
> Yes, but ad-hoc signing was introduced back in JDK 20 with 
> https://bugs.openjdk.org/browse/JDK-8298488 for macOS x64 and in JDK 19 with 
> https://bugs.openjdk.org/browse/JDK-8277493 for macOS aarch64. We do ad-hoc 
> signing for app images when generating PKG as well, so it is for all targets 
> and not only for app-image or dmg.
> 
> Thanks,
> Alexander

Ok, thanks again

Mike

Integrated: 8313904: [macos] All signing tests which verifies unsigned app images are failing

2023-08-11 Thread Alexander Matveev 
On Thu, 10 Aug 2023 22:58:18 GMT, Alexander Matveev  
wrote:

> - This is regression from 
> [JDK-8298488](https://bugs.openjdk.org/browse/JDK-8298488).
> - Since JDK-8298488 unsigned app bundles are ad-hoc signed and `codesign` 
> will report that app bundle is signed and thus our tests failed.
> - Fixed tests by checking that all app bundles are signed and by checking how 
> they signed ad-hoc vs actual certificate.
> - Unsigned post process image will be ad-hoc re-sign when generating DMG or 
> PKG, since we adding `.package` file which makes ad-hoc signature invalid. 
> This is similar to [JDK-8293462](https://bugs.openjdk.org/browse/JDK-8293462).

This pull request has now been integrated.

Changeset: ec0cc630
Author:Alexander Matveev 
URL:   
https://git.openjdk.org/jdk/commit/ec0cc6300a02dd92b25d9072b8b3859dab583bbd
Stats: 94 lines in 8 files changed: 58 ins; 0 del; 36 mod

8313904: [macos] All signing tests which verifies unsigned app images are 
failing

Reviewed-by: asemenyuk

-

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

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

2023-08-11 Thread Srinivas Vamsi Parasa 
On Thu, 15 Jun 2023 09:21:13 GMT, Laurent Bourgès  wrote:

>> * improved  mixed insertion sort (makes whole sorting faster)
>> * introduced Radix which sort shows several times boost of performance and 
>> has linear complexity instead of n*ln(n)
>> * improved merging sort for almost sorted data
>> * optimized parallel sorting
>> * improved step for pivot candidates and pivot partitioning
>> * extended existing tests
>> * added benchmarking JMH tests
>> * suggested better buffer allocation: if no memory, it is switched to 
>> in-place sorting with no OutOfMemoryError, threshold is 1/16th heap
>> 
>> I am going on previous PR by Vladimir Yaroslavskyi: 
>> https://github.com/openjdk/jdk/pull/3938
>
> Laurent Bourgès has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Merge branch 'dpqs23' of github.com:bourgesl/jdk-official into dpqs23
>  - simplified test to enable radix sort (improved sorting on period and 
> shuffle data) + updated version to 22

Hello, please see an implementation of x86 SIMD sort for Java here: 
https://github.com/openjdk/jdk/pull/14227

-

PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1675464124

Re: RFR: 8314169: Combine related RoundingMode logic in j.text.DigitList

2023-08-11 Thread Naoto Sato 
On Fri, 11 Aug 2023 18:27:47 GMT, Justin Lu  wrote:

> Please review this PR which is a broad clean up of the DigitList class (used 
> by Format classes in j.text).
> 
> This PR is intended to be a portion of a bigger change (split up to make 
> reviewing easier). 
> 
> The main change combines related Rounding Mode logic in `shouldRoundUp()` - 
> (_CEILING/FLOOR_, _HALF_UP/DOWN/EVEN_)
> 
> Other changes include
> - Certain for loops can be replaced with cleaner syntax (E.g. for(;;), empty 
> for loops)
> - Introduce overloaded `round(int)` - For use by Integer representations of 
> DigitList
> - Introduce `non0AfterIndex(int)` - To reduce code duplication

src/java.base/share/classes/java/text/DigitList.java line 123:

> 121:  * from the given index until the end.
> 122:  */
> 123: private boolean non0AfterIndex(int index) {

I'd prefer spelling out `0` to `Zero`.

src/java.base/share/classes/java/text/DigitList.java line 409:

> 407:  * Upon return, count will be less than or equal to maximumDigits.
> 408:  */
> 409: private void round(int maximumDigits) {

If the method is only used for `Long` and `BigInteger`, probably use the 
specific name instead of explaining it in the description and overloading would 
be more clear to me.

src/java.base/share/classes/java/text/DigitList.java line 521:

> 519: if (non0AfterIndex(maximumDigits)) {
> 520: return (isNegative && roundingMode == 
> RoundingMode.FLOOR)
> 521: || (!isNegative && roundingMode == 
> RoundingMode.CEILING);

`roundingMode` is checked against `FLOOR`/`CEILING` twice. I don't see the need 
of bundling them up.

src/java.base/share/classes/java/text/DigitList.java line 526:

> 524: case HALF_UP:
> 525: case HALF_DOWN:
> 526: case HALF_EVEN:

Fix the indentation

src/java.base/share/classes/java/text/DigitList.java line 543:

> 541: && (maximumDigits > 0) 
> && (digits[maximumDigits - 1] % 2 != 0));
> 542: // Not already rounded, and not exact, 
> round up
> 543: } else {

Are you sure this logic is equivalent to the previous one? Previously, 
`HALF_UP/DOWN` checks `valueExactAsDecimal` before `alreadyRounded`, but the 
new one checks `alreadyRounded` first.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291822984
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291829499
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291833892
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291834906
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291843188