Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v3]

[Philippe Marschall]

> Hello, newbie here
> 
> I picked JDK-8138732 to work on because it has a "starter" label and I 
> believe I understand what to do.
> 
> - I tried to update the copyright year to 2020 in every file.
> - I decided to change `@since` from 9 to 16 since it is a new annotation name 
> in a new package.
> - I tried to keep code changes to a minimum, eg not change to imports if 
> fully qualified class names are used instead of
>   imports. In some cases I did minor reordering of imports to keep them 
> sorted alphabetically.
> - All tier1 tests pass.
> - One jpackage/jlink tier2 test fails but I don't believe it is related.
> - Some tier3 Swing tests fail but I don't think they are related.

Philippe Marschall has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now
contains one commit:

  8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate

-

Changes: https://git.openjdk.java.net/jdk/pull/45/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=45=02
  Stats: 748 lines in 64 files changed: 149 ins; 149 del; 450 mod
  Patch: https://git.openjdk.java.net/jdk/pull/45.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/45/head:pull/45

PR: https://git.openjdk.java.net/jdk/pull/45

Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char)

[mknjc]

On Fri, 18 Sep 2020 23:11:46 GMT, Jason Tatton 
 wrote:

>> "the JVM has knowledge of the AVX capability of the chip it’s running on and 
>> disables the AVX2 code path for chips
>> which suffer from the performance degradation which has been outlined in 
>> this discussion"
>> Does it? The white paper Andrew cited doesn't mention this as being specific 
>> to only some chips that implement AVX2.
>> Can you explain where this restricted effect is documented?
>> Also, I assume you are referring to the code in vm_version_x86.cpp with this 
>> comment
>> 
>> // Don't use AVX-512 on older Skylakes unless explicitly requested
>> 
>> is that correct?
>
>> Can you explain where this restricted effect is documented?
> 
> Certainly! I’ve found that determining the capability of the CPU and whether 
> to enable AVX2 support if the chip
> supports it is mostly controlled in: [vm_version_x86.cpp](
> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp)
>  specifically:
> [get_processor_features](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L684-L755)
> and in [generate_get_cpu_info](
> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L69-L611).
>   In order to test the
> patch comprehensively I had to track down an Intel Core i7 (I7-9750H) 
> processor which the aforementioned code permitted
> AVX2 instructions for (maybe this is an error and it should not be enabled 
> for this processor though) as most of the
> infrastructure I personally use here at AWS runs on Intel Xeon processors - I 
> also tested on a E5-2680 which the JVM
> does not enable AVX2 for.  However, this is just the Intel side of things. 
> When it comes to AMD I read that the AMD Zen
> 2 architecture, of which the current flagship: Threadripper 3990X, is based, 
> is able to support AVX2 [without the
> frequency scaling](
> https://www.anandtech.com/Show/Index/14525?cPage=7=False=0=9=amd-zen-2-microarchitecture-analysis-ryzen-3000-and-epyc-rome)
> which some/all(?) of the Intel chips incur. I personally don’t have access to 
> one of these chips so I cannot confirm
> how it is classified in the JVM.  Also, I found when investigating this that 
> there is actually a JVM flag which can be
> used to control what level of AVX is enabled: `-XX:UseAVX=version.`

I really don't see the problem with using AVX for this?
As long as the used instructions all only activate license l0 the cpu don't 
scale the frequency at all. For AVX2 these
are most of all instructions which don't use the FMA or floating point ports. 
Additionally the cpu doesn't instant
scale down the frequency but runs the 256 bit instructions with reduced 
throughput but full cpu clock until enough
instructions use the avx command set. For more information see 
https://stackoverflow.com/a/56861355/130541

So as long the 512bit width instructions aren't used there should no frequency 
scaling happening.

-

PR: https://git.openjdk.java.net/jdk/pull/71

RFR 8253451: Performance regression in java.util.Scanner after 8236201

[Martin Balao]

Hi,

I'd like to propose a fix for JDK-8253451 [1] performance regression.

Webrev.00:

 * http://cr.openjdk.java.net/~mbalao/webrevs/8253451/8253451.webrev.00

As explained in [1], the idea for the fix is to optimize the regexp
string for the most common group and decimal separator characters across
locales. Please note that there are a few locales where the proposed
optimization does not apply and the slower -but safe- regexp introduced
by 8236201 is used.

Testing:

 * No regressions observed in jdk/java/util/Scanner (5/5 passed)

 * Verified performance improvement back to what it was before 8236201
(on my Americas locale).

Thanks,
Martin.-

--
[1] - https://bugs.openjdk.java.net/browse/JDK-8253451

Re: RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests [v2]

[Alexander Matveev]

> https://bugs.openjdk.java.net/browse/JDK-8231591
> 
> - Added MultiLauncherTwoPhaseTest which uses predefine app image with 
> multiple launcher and tests to make sure installer
>   will create shortcuts for all launchers.
> - Fixed Linux DesktopIntegration to create shortcuts for additional launcher 
> if we using pre-define app image.

Alexander Matveev has updated the pull request incrementally with one 
additional commit since the last revision:

  8231591: [TESTBUG] Create additional two phase jpackage tests (revision 1)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/263/files
  - new: https://git.openjdk.java.net/jdk/pull/263/files/e5254d22..5303fcc0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=263=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=263=00-01

  Stats: 107 lines in 5 files changed: 20 ins; 52 del; 35 mod
  Patch: https://git.openjdk.java.net/jdk/pull/263.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/263/head:pull/263

PR: https://git.openjdk.java.net/jdk/pull/263

Re: RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests

[Alexander Matveev]

On Mon, 21 Sep 2020 14:39:33 GMT, Alexey Semenyuk  wrote:

> How about testing of other jpackage command line options in two phase mode? 
> Like "--name", "--version"? Any plans to
> add them?

Plan was to file separate bugs for any additional options. I do not think we 
should put everything in one issue. This
issue for desktop integration for additional launchers.

> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java line 140:
> 
>> 138: cmd.verifyIsOfType(PackageType.WINDOWS);
>> 139: this.cmd = cmd;
>> 140: this.name = name;
> 
> I'd suggest to have the following initialization of "name" field:
> `this.name = (name == null ? cmd.name() : name)`
> This will help to avoid multiple `(name == null ? cmd.name() : name)` in the 
> code.

Fixed.

> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 54:
> 
>> 52: public class MultiLauncherTwoPhaseTest {
>> 53:
>> 54: public static void main(String[] args) throws Exception {
> 
> Please replace main() function with dedicated test function. You can use 
> SimplePackageTest as a template, This would
> give better control on how to run the test in debug mode.

Fixed.

> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 56:
> 
>> 54: public static void main(String[] args) throws Exception {
>> 55: TKit.run(args, () -> {
>> 56: Path appimageOutput = TKit.workDir().resolve("appimage");
> 
> Please use TKit.createTempDirectory() to create directories for intermediate 
> data of test runs. This allows clean
> multiple runs of the test in the same work directory.

Fixed.

> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 80:
> 
>> 78: })
>> 79: .addInstallVerifier(cmd -> {
>> 80: launcher1.verify(cmd);
> 
> There is no need in explicit call to AdditionalLauncher.verify() as this 
> function is scheduled for the call from
> AdditionalLauncher.applyTo() call.  Thus there is no need to change access to 
> this function from "private" to "public"
> in your patch to AdditionalLauncher.java.

Fixed.

> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java line 559:
> 
>> 557: && !cmd.isPackageUnpacked(
>> 558: "Not verifying desktop integration")) {
>> 559: new WindowsHelper.DesktopIntegrationVerifier(cmd, 
>> null);
> 
> I'd rather add a loop calling DesktopIntegrationVerifier() for all launchers 
> than copy/paste the code in
> AdditionalLauncher.java

Fixed. I also removed code from AdditionalLaunchers.

-

PR: https://git.openjdk.java.net/jdk/pull/263

Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char)

[Vladimir Kozlov]

On Mon, 21 Sep 2020 09:20:56 GMT, Andrew Dinn  wrote:

>>> Can you explain where this restricted effect is documented?
>> 
>> Certainly! I’ve found that determining the capability of the CPU and whether 
>> to enable AVX2 support if the chip
>> supports it is mostly controlled in: [vm_version_x86.cpp](
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp)
>>  specifically:
>> [get_processor_features](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L684-L755)
>> and in [generate_get_cpu_info](
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L69-L611).
>>   In order to test the
>> patch comprehensively I had to track down an Intel Core i7 (I7-9750H) 
>> processor which the aforementioned code permitted
>> AVX2 instructions for (maybe this is an error and it should not be enabled 
>> for this processor though) as most of the
>> infrastructure I personally use here at AWS runs on Intel Xeon processors - 
>> I also tested on a E5-2680 which the JVM
>> does not enable AVX2 for.  However, this is just the Intel side of things. 
>> When it comes to AMD I read that the AMD Zen
>> 2 architecture, of which the current flagship: Threadripper 3990X, is based, 
>> is able to support AVX2 [without the
>> frequency scaling](
>> https://www.anandtech.com/Show/Index/14525?cPage=7=False=0=9=amd-zen-2-microarchitecture-analysis-ryzen-3000-and-epyc-rome)
>> which some/all(?) of the Intel chips incur. I personally don’t have access 
>> to one of these chips so I cannot confirm
>> how it is classified in the JVM.  Also, I found when investigating this that 
>> there is actually a JVM flag which can be
>> used to control what level of AVX is enabled: `-XX:UseAVX=version.`
>
>>  >Can you explain where this restricted effect is documented?
> 
>> Certainly! I’ve found that determining the capability of the CPU and whether 
>> to enable AVX2 support if the chip
>> supports it is mostly controlled in: vm_version_x86.cpp specifically: 
>> get_processor_features and in
>> generate_get_cpu_info.
> 
> Yes, I can see what the code does. I was asking where the cpu behaviour is 
> documented independent of the code.
> 
>> In order to test the patch comprehensively I had to track down an Intel Core 
>> i7 (I7-9750H) processor which the
>> aforementioned code permitted AVX2 instructions for (maybe this is an error 
>> and it should not be enabled for this
>> processor though) as most of the infrastructure I personally use here at AWS 
>> runs on Intel Xeon processors - I also
>> tested on a E5-2680 which the JVM does not enable AVX2 for.
> 
> 'maybe'? The documentation Andrew provided mentioned Xeon E5 v3 which I 
> believe is a Skylake design. However, the code
> I pointed you at in vm_version_x86 which claims to detect 'early Skylake' 
> designs is only disabling AVX512 support. It
> still enables AVX2. Similarly, the code that generates machine code to check 
> the processor capabilities has a special
> check if use_evex is set (i.e. AVX3 is requested)  which disables AVX512 for 
> Skylake but does not disable AVX2 support.
>> However, this is just the Intel side of things. When it comes to AMD I read 
>> that the AMD Zen 2 architecture, of which
>> the current flagship: Threadripper 3990X, is based, is able to support AVX2 
>> without the frequency scaling which
>> some/all(?) of the Intel chips incur. I personally don’t have access to one 
>> of these chips so I cannot confirm how it
>> is classified in the JVM.
> 
> Well, it would be good to know where you read that and to see if that 
> confirms thar the code is avoiding the issue
> Andrew raised.
>> Also, I found when investigating this that there is actually a JVM flag 
>> which can be used to control what level of AVX
>> is enabled: -XX:UseAVX=version.
> 
> Yes, indeed. However, what I am trying to understand is whether the current 
> code is bypassing the problem Andrew
> brought up in the cases where that problem actually exists. It doesn't look 
> like it so far given that the problem
> applies to AVX2 and only AVX512 support is being disabled and, even then only 
> for some (Skylake) processors. Without
> some clear documentation of what processors suffer from this power surge 
> problem it will not be possible to decide
> whether this patch is doing the right thing or not.

Based on comment by @jatin-bhateja (Intel) frequency level switchover pointed 
by @theRealAph  is sensitive to vector
size https://github.com/openjdk/jdk/pull/144#issuecomment-692044896

By keeping vector size less or equal to 32 bytes we should avoid it. And as I 
can see this intrinsic code is using 32
bytes (chars) and 16 bytes vectors: `pbroadcastb(vec1, vec1, 
Assembler::AVX_256bit);`

Also we never had issues with AVX2. only with AVX512 regarding performance hit:
https://bugs.openjdk.java.net/browse/JDK-8221092

I would like to see performance numbers for for all values of UseAVX flag : 0, 
1, 2, 

Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

[Serguei Spitsyn]

On Tue, 22 Sep 2020 00:38:43 GMT, Serguei Spitsyn  wrote:

>> Reviewed in the good old hg times :)
>> See also 
>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041453.html
>
> Hi Richard,
> 
> Could you consider to place the classes EscapeBarrier and 
> JvmtiDeferredUpdates into theyr own .hpp/.cpp files? The
> class JvmtiDeferredUpdates would be better to put into the folder 'prims' 
> then.
> src/hotspot/share/opto/macro.cpp:
> @@ -1091,11 +1091,11 @@
>  bool PhaseMacroExpand::eliminate_allocate_node(AllocateNode *alloc) {
>// Don't do scalar replacement if the frame can be popped by JVMTI:
>// if reallocation fails during deoptimization we'll pop all
>// interpreter frames for this compiled frame and that won't play
>// nice with JVMTI popframe.
> -  if (!EliminateAllocations || JvmtiExport::can_pop_frame() || 
> !alloc->_is_non_escaping) {
> +  if (!EliminateAllocations || !alloc->_is_non_escaping) {
>  return false;
>}
>   
> I wonder if the comment is still correct after you removed the check for 
> JvmtiExport::can_pop_frame().
> 
> src/hotspot/share/runtime/deoptimization.hpp:
> 
> +  EscapeBarrier(JavaThread* calling_thread, JavaThread* deoptee_thread, bool 
> barrier_active)
> +: _calling_thread(calling_thread), _deoptee_thread(deoptee_thread),
> +  _barrier_active(barrier_active && (JVMCI_ONLY(UseJVMCICompiler) 
> NOT_JVMCI(false)
> +  COMPILER2_PRESENT(|| DoEscapeAnalysis)))
> . . . . . . . . .
> +
> +  // Revert ea based optimizations for all java threads
> +  EscapeBarrier(JavaThread* calling_thread, bool barrier_active)
> +: _calling_thread(calling_thread), _deoptee_thread(NULL),
> 
> Nit: would better to make the parameter deoptee_thread to be the 3rd to 
> better mach the seconf constructor.
> 
> +  bool all_threads()const { return _deoptee_thread == NULL; }
> // Should revert optimizations for all
> threads. +  bool self_deopt() const { return _calling_thread == 
> _deoptee_thread; } // Current thread deoptimizes
> its own objects. +  bool barrier_active() const { return _barrier_active; }   
>  // Inactive barriers are
> created if no local objects can escape.
> I'd suggest to put comments in a line before function definitions as it is 
> done for other declarations/definitions.

src/hotspot/share/runtime/deoptimization.cpp:
 @@ -349,12 +408,12 @@
 
   // Now that the vframeArray has been created if we have any deferred local 
writes
   // added by jvmti then we can free up that structure as the data is now in 
the
   // vframeArray
 
-  if (thread->deferred_locals() != NULL) {
-GrowableArray* list = 
thread->deferred_locals();
+  if (JvmtiDeferredUpdates::deferred_locals(thread) != NULL) {
+GrowableArray* list = 
JvmtiDeferredUpdates::deferred_locals(thread);
 int i = 0;
 do {
   // Because of inlining we could have multiple vframes for a single frame
   // and several of the vframes could have deferred writes. Find them all.
   if (list->at(i)->id() == array->original().id()) {

@@ -365,13 +424,14 @@
   } else {
 i++;
   }
 } while ( i < list->length() );
 if (list->length() == 0) {
-  thread->set_deferred_locals(NULL);
-  // free the list and elements back to C heap.
-  delete list;
+  JvmtiDeferredUpdates* updates = thread->deferred_updates();
+  thread->set_deferred_updates(NULL);
+  // free deferred updates.
+  delete updates;
 }
It is not clear why the 'list' is not deleted anymore. If it is intentional 
then could you, please, add a comment with
an explanation?

-

PR: https://git.openjdk.java.net/jdk/pull/119

Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v11]

[Ian Graves]

> Related to [JDK-8252730 jlink does not create reproducible builds on different
> servers](https://bugs.openjdk.java.net/browse/JDK-8252730). Introduces 
> ordering based on `Archive` module names to
> ensure stable files (and file signatures) across generated JDK images by 
> jlink.

Ian Graves has updated the pull request incrementally with one additional 
commit since the last revision:

  Cleanup on Test 3

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/156/files
  - new: https://git.openjdk.java.net/jdk/pull/156/files/8faf1fda..3cf1498e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=156=10
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=156=09-10

  Stats: 37 lines in 1 file changed: 6 ins; 17 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/156.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/156/head:pull/156

PR: https://git.openjdk.java.net/jdk/pull/156

Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v9]

[Ian Graves]

On Mon, 21 Sep 2020 15:52:15 GMT, Alan Bateman  wrote:

>> 1. I agree that it's awkward and I'm not a huge fan. I'm okay doing it this 
>> way, but this is recreating the reproducer
>> for https://bugs.openjdk.java.net/browse/JDK-8252730 which is two identical 
>> JDK's in different directories. I can
>> document this in the test further or we can change it.  2-4: Noted. Will 
>> clean these up and loop back. Thanks!
>
> Whatever is easiest but 2 x copy tree would be simpler and the result should 
> be two identical JDKs as reported in
> JDK-8252730.

I misunderstood what you meant when I read it earlier today, apologies! The new 
changes should address these enumerated
issues.

-

PR: https://git.openjdk.java.net/jdk/pull/156

Re: RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

[Serguei Spitsyn]

On Thu, 17 Sep 2020 07:45:52 GMT, Goetz Lindenmaier  wrote:

>> Marked as reviewed by goetz (Reviewer).
>
> Reviewed in the good old hg times :)
> See also 
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041453.html

src/hotspot/share/opto/macro.cpp:

@@ -1091,11 +1091,11 @@
 bool PhaseMacroExpand::eliminate_allocate_node(AllocateNode *alloc) {
   // Don't do scalar replacement if the frame can be popped by JVMTI:
   // if reallocation fails during deoptimization we'll pop all
   // interpreter frames for this compiled frame and that won't play
   // nice with JVMTI popframe.
-  if (!EliminateAllocations || JvmtiExport::can_pop_frame() || 
!alloc->_is_non_escaping) {
+  if (!EliminateAllocations || !alloc->_is_non_escaping) {
 return false;
   }
  
I wonder if the comment is still correct after you removed the check for 
JvmtiExport::can_pop_frame().

-

PR: https://git.openjdk.java.net/jdk/pull/119

Re: RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests

[Alexander Matveev]

On Mon, 21 Sep 2020 14:05:08 GMT, Alexey Semenyuk  wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8231591
>> 
>> - Added MultiLauncherTwoPhaseTest which uses predefine app image with 
>> multiple launcher and tests to make sure installer
>>   will create shortcuts for all launchers.
>> - Fixed Linux DesktopIntegration to create shortcuts for additional launcher 
>> if we using pre-define app image.
>
> src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/DesktopIntegration.java
>  line 549:
> 
>> 547: private final List associations;
>> 548:
>> 549: private static boolean initAppImageLaunchers = true;
> 
> What is the logic behind static variable?

It is used to avoid infinite calls in DesktopIntegration(). I modified it by 
removing PREDEFINED_APP_IMAGE from
launcherParams.

-

PR: https://git.openjdk.java.net/jdk/pull/263

Re: RFR: 8246774: Record Classes (final) implementation [v2]

[Vicente Romero]

> Co-authored-by: Vicente Romero 
> Co-authored-by: Harold Seigel 
> Co-authored-by: Jonathan Gibbons 
> Co-authored-by: Brian Goetz 
> Co-authored-by: Maurizio Cimadamore 
> Co-authored-by: Joe Darcy 
> Co-authored-by: Chris Hegarty 
> Co-authored-by: Jan Lahoda 

Vicente Romero has updated the pull request incrementally with one additional 
commit since the last revision:

  removing the javax.lang.model related code to be moved to a separate bug

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/290/files
  - new: https://git.openjdk.java.net/jdk/pull/290/files/9eedb3ab..543e5054

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=290=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=290=00-01

  Stats: 134 lines in 12 files changed: 130 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/290.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/290/head:pull/290

PR: https://git.openjdk.java.net/jdk/pull/290

Re: RFR: 8253149: Building an installer from invalid app image fails on Window… [v2]

[Alexey Semenyuk]

On Mon, 21 Sep 2020 19:36:03 GMT, Andy Herrick  wrote:

>> 8253149: Building an installer from invalid app image fails on Windows and 
>> Linux
>> When jpackage builds a package from an app-image that was not generated by 
>> jpackage, the tool should give user a
>> warning message, and then complete the package anyway.
>
> Andy Herrick has updated the pull request 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:
>  - 8253149: Building an installer from invalid app image fails (revision 1)
>  - Merge branch 'master' into JDK-8253149
>  - 8253149: Building an installer from invalid app image fails on Windows and 
> Linux

test/jdk/tools/jpackage/share/AppImagePackageTest.java line 26:

> 24: import java.nio.file.Path;
> 25: import java.nio.file.Files;
> 26: import java.io.BufferedWriter;

Seems like this import is not needed any more.

src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/resources/LinuxResources.properties
 line 64:

> 62: message.rpm-ldd-not-available.advice=Install "glibc-common" RPM package 
> to get ldd.
> 63:
> 64: warning.forign-app-image=Warning: app-image dir not generated by jpackage.

Should it be "foreign" instead of "forign"?

-

PR: https://git.openjdk.java.net/jdk/pull/271

Re: RFR: 8246774: Record Classes (final) implementation

[Vicente Romero]

On Mon, 21 Sep 2020 21:53:05 GMT, Joe Darcy  wrote:

>> Please review the fix for [1]. The intention of this patch is to make 
>> records final removing the need to
>> use --enable-preview in order to be able to include a record declaration in 
>> a source or for the VM to execute code
>> compiled from record classes,  Thanks
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8246774
>
> Hi Vicente,
> Please file a separate subtask for the javax.lang.model changes. This helps 
> with the JSR 269 MR paperwork.
> Thanks,
> -Joe

note: I have removed from the original patch the code related to 
javax.lang.model, I will publish them in a separate PR

-

PR: https://git.openjdk.java.net/jdk/pull/290

Re: RFR: 8253208: Move CDS related code to a separate class [v3]

[Yumin Qi]

> With more CDS related code added to VM, it is time to move CDS code to a 
> separate class. CDS is the new class which is
> specific to CDS.
> Tests: tier1-4

Yumin Qi has updated the pull request incrementally with one additional commit 
since the last revision:

  8253208: Move CDS related code to a separate class

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/261/files
  - new: https://git.openjdk.java.net/jdk/pull/261/files/c01deec7..953b6bac

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=261=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=261=01-02

  Stats: 10 lines in 1 file changed: 9 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/261.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/261/head:pull/261

PR: https://git.openjdk.java.net/jdk/pull/261

Re: RFR: 8253208: Move CDS related code to a separate class [v2]

[Ioi Lam]

On Mon, 21 Sep 2020 18:17:55 GMT, Yumin Qi  wrote:

>> With more CDS related code added to VM, it is time to move CDS code to a 
>> separate class. CDS is the new class which is
>> specific to CDS.
>> Tests: tier1-4
>
> Yumin Qi has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8253208: Move CDS related code to a separate class

Marked as reviewed by iklam (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/261

Re: RFR: 8246774: Record Classes (final) implementation

[Joe Darcy]

On Mon, 21 Sep 2020 21:36:39 GMT, Vicente Romero  wrote:

>> Co-authored-by: Vicente Romero 
>> Co-authored-by: Harold Seigel 
>> Co-authored-by: Jonathan Gibbons 
>> Co-authored-by: Brian Goetz 
>> Co-authored-by: Maurizio Cimadamore 
>> Co-authored-by: Joe Darcy 
>> Co-authored-by: Chris Hegarty 
>> Co-authored-by: Jan Lahoda 
>
> Please review the fix for [1]. The intention of this patch is to make records 
> final removing the need to
> use --enable-preview in order to be able to include a record declaration in a 
> source or for the VM to execute code
> compiled from record classes,  Thanks
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8246774

Hi Vicente,
Please file a separate subtask for the javax.lang.model changes. This helps 
with the JSR 269 MR paperwork.
Thanks,
-Joe

-

PR: https://git.openjdk.java.net/jdk/pull/290

Re: RFR: 8246774: Record Classes (final) implementation

[Vicente Romero]

On Mon, 21 Sep 2020 21:30:51 GMT, Vicente Romero  wrote:

> Co-authored-by: Vicente Romero 
> Co-authored-by: Harold Seigel 
> Co-authored-by: Jonathan Gibbons 
> Co-authored-by: Brian Goetz 
> Co-authored-by: Maurizio Cimadamore 
> Co-authored-by: Joe Darcy 
> Co-authored-by: Chris Hegarty 
> Co-authored-by: Jan Lahoda 

Please review the fix for [1]. The intention of this patch is to make records 
final removing the need to
use --enable-preview in order to be able to include a record declaration in a 
source or for the VM to execute code
compiled from record classes,

Thanks

[1] https://bugs.openjdk.java.net/browse/JDK-8246774

-

PR: https://git.openjdk.java.net/jdk/pull/290

RFR: 8246774: Record Classes (final) implementation

[Vicente Romero]

Co-authored-by: Vicente Romero 
Co-authored-by: Harold Seigel 
Co-authored-by: Jonathan Gibbons 
Co-authored-by: Brian Goetz 
Co-authored-by: Maurizio Cimadamore 
Co-authored-by: Joe Darcy 
Co-authored-by: Chris Hegarty 
Co-authored-by: Jan Lahoda 

-

Commit messages:
 - 8246774: Record Classes (final) implementation

Changes: https://git.openjdk.java.net/jdk/pull/290/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=290=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8246774
  Stats: 495 lines in 95 files changed: 23 ins; 362 del; 110 mod
  Patch: https://git.openjdk.java.net/jdk/pull/290.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/290/head:pull/290

PR: https://git.openjdk.java.net/jdk/pull/290

Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v10]

[Ian Graves]

> Related to [JDK-8252730 jlink does not create reproducible builds on different
> servers](https://bugs.openjdk.java.net/browse/JDK-8252730). Introduces 
> ordering based on `Archive` module names to
> ensure stable files (and file signatures) across generated JDK images by 
> jlink.

Ian Graves has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now
contains 11 commits:

 - Using File.walk for copy
 - Cleanup extraneous directory ops
 - Changes to test 3 so correct JDK under test is run
 - Test 3 to non-tmp directory
 - Merging two tests and renaming a test
 - Merging tests and renaming test 4
 - Copying test jdk to test consistency
 - Test updates
 - Comparator cleanup
 - Merging streams
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/2e30ff61...8faf1fda

-

Changes: https://git.openjdk.java.net/jdk/pull/156/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=156=09
  Stats: 134 lines in 3 files changed: 126 ins; 1 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/156.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/156/head:pull/156

PR: https://git.openjdk.java.net/jdk/pull/156

Re: RFR: 8253149: Building an installer from invalid app image fails on Window… [v2]

[Andy Herrick]

> 8253149: Building an installer from invalid app image fails on Windows and 
> Linux
> When jpackage builds a package from an app-image that was not generated by 
> jpackage, the tool should give user a
> warning message, and then complete the package anyway.

Andy Herrick has updated the pull request 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:

 - 8253149: Building an installer from invalid app image fails (revision 1)
 - Merge branch 'master' into JDK-8253149
 - 8253149: Building an installer from invalid app image fails on Windows and 
Linux

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/271/files
  - new: https://git.openjdk.java.net/jdk/pull/271/files/6c089a95..e3f8a8fa

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=271=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=271=00-01

  Stats: 12895 lines in 400 files changed: 7857 ins; 3987 del; 1051 mod
  Patch: https://git.openjdk.java.net/jdk/pull/271.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/271/head:pull/271

PR: https://git.openjdk.java.net/jdk/pull/271

Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v3]

[Lance Andersen]

On Mon, 21 Sep 2020 12:18:40 GMT, Alan Bateman  wrote:

> Update to Deflater.c looks good.
> 
> DeflaterDictionaryTests looks like is a shaping up to be a good test for 
> setDictionary. Are there other assertions that
> should be checked, e.g. setDictionary(ByteBuffer) is specified to consume all 
> bytes and it would be good to check that
> the position is set to the limit. Also can the 2-arg setDictionary be tests, 
> also corner cases such no bytes remaining
> or invoked with null.

I have logged JDK-8253444 to add more coverage for setDictionary.  I would 
prefer to put this change back to address
the offset not being used and improve the coverage afterwards.

> The naming of the tests is a bit inconsistent, e.g. ByteArrayTest vs. 
> testByteBufferDirect. In the naming then I would
> use "Heap" instead of "Wrap", as in testHeapByteBuffer.

I  updated the test name(s).
> 
> In passing: The tests can use try-finally to ensure that Deflater::end is 
> invoked even when the test fails. String
> repeat(int) could be used to avoid duplicating "Welcome to Us Open;". Missing 
> space in several "if(...)" usages.

Addressed the issues above.

I have pushed the updated changes for review to the PR.

-

PR: https://git.openjdk.java.net/jdk/pull/269

Re: RFR: 8253208: Move CDS related code to a separate class [v2]

[Erik Joelsson]

On Mon, 21 Sep 2020 18:17:55 GMT, Yumin Qi  wrote:

>> With more CDS related code added to VM, it is time to move CDS code to a 
>> separate class. CDS is the new class which is
>> specific to CDS.
>> Tests: tier1-4
>
> Yumin Qi has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8253208: Move CDS related code to a separate class

Build changes look good.

-

PR: https://git.openjdk.java.net/jdk/pull/261

Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v4]

[Lance Andersen]

> Hi all,
> 
> Please review the fix  for JDK-8252739 which addresses an issue introduced by
> https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored 
> the offset specified by
> Deflater.setDictionary.  Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly 
> as well as the java/util/zip and
> java/util/jar JCK tests.

Lance Andersen has updated the pull request incrementally with one additional 
commit since the last revision:

  More  updates to testByteBufferDirect in DeflaterDictionaryTests.java

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/269/files
  - new: https://git.openjdk.java.net/jdk/pull/269/files/aac03e3e..f34a6702

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=269=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=269=02-03

  Stats: 172 lines in 1 file changed: 72 ins; 69 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/269.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/269/head:pull/269

PR: https://git.openjdk.java.net/jdk/pull/269

Re: RFR: 8253208: Move CDS related code to a separate class [v2]

[Mandy Chung]

On Mon, 21 Sep 2020 18:17:55 GMT, Yumin Qi  wrote:

>> With more CDS related code added to VM, it is time to move CDS code to a 
>> separate class. CDS is the new class which is
>> specific to CDS.
>> Tests: tier1-4
>
> Yumin Qi has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8253208: Move CDS related code to a separate class

This patch looks okay.   Please add the javadoc for 
`CDS::getRandomSeedForDumping` and `CDS::defineArchiveModules` for
completeness.

-

PR: https://git.openjdk.java.net/jdk/pull/261

Re: RFR: 8253208: Move CDS related code to a separate class [v2]

[Yumin Qi]

On Sun, 20 Sep 2020 06:10:53 GMT, Ioi Lam  wrote:

>> Yumin Qi has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8253208: Move CDS related code to a separate class
>
> src/java.base/share/native/libjava/CDS.c line 49:
> 
>> 47: JNIEXPORT jboolean JNICALL
>> 48: Java_jdk_internal_misc_CDS_isCDSDumpingEnabled(JNIEnv *env, jclass jcls) 
>> {
>> 49: return JVM_IsCDSDumpingEnabled(env);
> 
> Maybe: return JVM_IsCDSDynamicDumpingEnabled(env)

updated with comments.

-

PR: https://git.openjdk.java.net/jdk/pull/261

Re: RFR: 8253208: Move CDS related code to a separate class [v2]

[Yumin Qi]

> With more CDS related code added to VM, it is time to move CDS code to a 
> separate class. CDS is the new class which is
> specific to CDS.
> Tests: tier1-4

Yumin Qi has updated the pull request incrementally with one additional commit 
since the last revision:

  8253208: Move CDS related code to a separate class

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/261/files
  - new: https://git.openjdk.java.net/jdk/pull/261/files/b9789c8b..c01deec7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=261=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=261=00-01

  Stats: 27 lines in 7 files changed: 0 ins; 0 del; 27 mod
  Patch: https://git.openjdk.java.net/jdk/pull/261.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/261/head:pull/261

PR: https://git.openjdk.java.net/jdk/pull/261

Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v9]

[Alan Bateman]

On Mon, 21 Sep 2020 14:17:18 GMT, Ian Graves  wrote:

>> test/jdk/tools/jlink/JLinkReproducible3Test.java line 112:
>> 
>>> 110: }
>>> 111: }
>>> 112:
>> 
>> The update to ImageFileCreator looks good.
>> 
>> A few nits in JLinkReproducible3Test:
>> 1. Invoking copyJDKs with two destination locations looks a bit strange. I 
>> think would be easier for future maintainers
>> if it does one copy and is called twice. 2. Files.copy will create a 
>> directory, no need for Files.createDirectories.
>> 3. Are you sure that bin/jlink is executable, I just wonder if the test runs 
>> with the patch have ended up depending the
>> umask at the time. It is possible to specify COPY_ATTRIBUTES to the copy 
>> method and it will attempt to copy the
>> permissions. 4. The variable naming is more like C style, so a bit 
>> inconsistent with the naming usually used in Java
>> programs.
>
> 1. I agree that it's awkward and I'm not a huge fan. I'm okay doing it this 
> way, but this is recreating the reproducer
> for https://bugs.openjdk.java.net/browse/JDK-8252730 which is two identical 
> JDK's in different directories. I can
> document this in the test further or we can change it.  2-4: Noted. Will 
> clean these up and loop back. Thanks!

Whatever is easiest but 2 x copy tree would be simpler and the result should be 
two identical JDKs as reported in
JDK-8252730.

-

PR: https://git.openjdk.java.net/jdk/pull/156

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v3]

[Monica Beckwith]

> This is a continuation of 
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>  
> Changes since then:
> * We've improved the write barrier as suggested by Andrew [1]
> * The define-guards around R18 have been changed to `R18_RESERVED`. This will 
> be enabled for Windows only for now but
>   will be required for the upcoming macOS+Aarch64 [2] port as well.
> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov 
> in our PR for now and built the
>   Windows-specific CPU feature detection on top of it.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html
> [2] https://openjdk.java.net/jeps/8251280

Monica Beckwith has refreshed the contents of this pull request, and previous 
commits have been removed. The
incremental views will show differences compared to the previous content of the 
PR. The pull request contains 11 new
commits since the last revision:

 - 8248787: G1: Workaround MSVC bug
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248670: Windows: Exception handling support on AArch64
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248660: AArch64: Make _clear_cache and _nop portable
   Summary: __builtin___clear_cache, etc.
   Contributed-by: mbeckwit, luhenry, burban
 - 8248659: AArch64: Extend CPU Feature detection
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248656: Add Windows AArch64 platform support code
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248498: Add build system support for Windows AArch64
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248681: AArch64: MSVC doesn't support __PRETTY_FUNCTION__
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248663: AArch64: Avoid existing macros/keywords of MSVC
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248676: AArch64: Add workaround for LITable constructor
   Reviewed-by: aph
   Contributed-by: mbeckwit, luhenry, burban
 - 8248500: AArch64: Remove the r18 dependency on Windows AArch64 (regenerate 
tests)
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/5f9b0d35...3881b12d

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/212/files
  - new: https://git.openjdk.java.net/jdk/pull/212/files/5f9b0d35..3881b12d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=212=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=212=01-02

  Stats: 1366 lines in 2 files changed: 6 ins; 4 del; 1356 mod
  Patch: https://git.openjdk.java.net/jdk/pull/212.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/212/head:pull/212

PR: https://git.openjdk.java.net/jdk/pull/212

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v3]

[Bernhard Urban-Forster]

On Mon, 21 Sep 2020 08:15:20 GMT, Bernhard Urban-Forster  
wrote:

>> Hey @erikj79, thank you so much for giving it a try!
>> 
>>> Our linux-aarch64 build fails with this:
>>> cc: error: unrecognized command line option '-std=c++14'
>>> when compiling 
>>> build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch
>> 
>> Hmm, that's interesting. What environment is that exactly? What `configure` 
>> line are you using there? We have tested on
>> such a system: $ cat /etc/issue
>> Ubuntu 19.10 \n \l
>> $ bash configure --with-boot-jdk=/home/beurba/work/jdk-16+13 --with-jtreg
>> $ make clean CONF=linux-aarch64-server-release
>> $ make images JOBS=255 LOG=info CONF=linux-aarch64-server-release
>> $ ./build/linux-aarch64-server-release/images/jdk/bin/java 
>> -XshowSettings:properties -version 2>&1 | grep aarch64
>> java.home = 
>> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk
>> os.arch = aarch64
>> sun.boot.library.path = 
>> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk/lib
>> 
>>> I'm trying to configure a windows-aarch64 build, but it fails on fixpath. 
>>> Is this something you are also experiencing,
>>> and if so, how are you addressing it?
>> 
>> Yes. As far as I understand, the problem is that `fixpath.exe` isn't built 
>> properly when doing cross-compiling on
>> Windows targets (as it hasn't been a thing so far). We use a workaround 
>> internally
>> https://gist.github.com/lewurm/c099a4b5fcd8a182510cbdeebcb41f77 , but a 
>> proper solution is under discussion on
>> build-dev: 
>> https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html
>
>> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on 
>> [build-dev](mailto:build-...@openjdk.java.net):_
>> 
>> On 18/09/2020 11:14, Monica Beckwith wrote:
>> 
>> > This is a continuation of 
>> > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>> 
>> The diffs in assembler_aarch64.cpp are mostly spurious. Please try this.
> 
> 
> Thank you Andrew. Is the goal to reduce the patch diff in 
> `assembler_aarch64.cpp`? If so, we need to get rid of the
> retry in your patch to avoid additional calls to `random`, e.g. something 
> like this (the diff for the generated part
> would look indeed nicer with that:  
> https://gist.github.com/lewurm/2e7b0e00447696c75e00febb83034ba1 ):
> --- a/src/hotspot/cpu/aarch64/aarch64-asmtest.py
> +++ b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
> @@ -13,6 +13,8 @@ class Register(Operand):
> 
>  def generate(self):
>  self.number = random.randint(0, 30)
> +if self.number == 18:
> +self.number = 17
>  return self
> 
>  def astr(self, prefix):
> @@ -37,6 +39,8 @@ class GeneralRegisterOrZr(Register):
> 
>  def generate(self):
>  self.number = random.randint(0, 31)
> +if self.number == 18:
> +self.number = 16
>  return self
> 
>  def astr(self, prefix = ""):
> @@ -54,6 +58,8 @@ class GeneralRegisterOrZr(Register):
>  class GeneralRegisterOrSp(Register):
>  def generate(self):
>  self.number = random.randint(0, 31)
> +if self.number == 18:
> +self.number = 15
>  return self
> 
>  def astr(self, prefix = ""):
> @@ -1331,7 +1337,7 @@ generate(SpecialCases, [["ccmn",   "__ ccmn(zr, zr, 3u, 
> Assembler::LE);",
>  ["st1w",   "__ sve_st1w(z0, __ S, p1, Address(r0, 
> 7));", "st1w\t{z0.s}, p1, [x0, #7, MUL VL]"],
>  ["st1b",   "__ sve_st1b(z0, __ B, p2, Address(sp, 
> r1));","st1b\t{z0.b}, p2, [sp, x1]"],
>  ["st1h",   "__ sve_st1h(z0, __ H, p3, Address(sp, 
> r8));","st1h\t{z0.h}, p3, [sp, x8, LSL #1]"],
> -["st1d",   "__ sve_st1d(z0, __ D, p4, Address(r0, 
> r18));",   "st1d\t{z0.d}, p4, [x0, x18, LSL #3]"],
> +["st1d",   "__ sve_st1d(z0, __ D, p4, Address(r0, 
> r17));",   "st1d\t{z0.d}, p4, [x0, x17,
> LSL #3]"],
>  ["ldr","__ sve_ldr(z0, Address(sp));",   
> "ldr\tz0, [sp]"],
>  ["ldr","__ sve_ldr(z31, Address(sp, -256));",
> "ldr\tz31, [sp, #-256, MUL VL]"],
>  ["str","__ sve_str(z8, Address(r8, 255));",  
> "str\tz8, [x8, #255, MUL VL]"],

> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on 
> [build-dev](mailto:build-...@openjdk.java.net):_
> 
> On 21/09/2020 09:18, Bernhard Urban-Forster wrote:
> 
> > Thank you Andrew. Is the goal to reduce the patch diff in 
> > `assembler_aarch64.cpp`? If so, we need to get rid of the
> > retry in your patch to avoid additional calls to `random`, e.g. something 
> > like this (the diff for the generated part
> > would look indeed nicer with that:  
> > 

Re: RFR: 8245194: Unix domain socket channel implementation [v5]

[Michael McMahon]

> Continuing this review as a PR on github with the comments below 
> incorporated. I expect there will be a few more
> iterations before integrating.
> On 06/09/2020 19:47, Alan Bateman wrote:
>> On 26/08/2020 15:24, Michael McMahon wrote:
>>>
>>> As I mentioned the other day, I wasn't able to use the suggested method on 
>>> Windows which returns an absolute path. So,
>>> I added a method to WindowsPath which returns the path in the expected 
>>> UTF-8 encoding as a byte[]. Let me know what you
>>> think of that.
>>>
>>> There is a fair bit of other refactoring and simplification done also. Next 
>>> version is at:
>>>
>>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/
>>>
>> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I 
>> don't think we should be caching it as the
>> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about 
>> encoding a relative path when the resolved
>> path (before encoding) is > 247 chars. The documentation on the MS site 
>> isn't very completely and I think there are a
>> number points that require clarification to fully understand how this will 
>> work with relative, directly relative and
>> drive relative paths.
>>
> 
> Maybe it would be better to just use the path returned from toString() and do 
> the conversion to UTF-8 externally. That
> would leave WindowsPath unchanged.
>> In the same area, the new PathUtil is a bit inconsistent with the existing 
>> provider code. One suggestion is to add a
>> method to AbstractFileSystemProvider instead. That is the base class the 
>> platform providers and would be a better place
>> to get the file path in bytes.
>>
> 
> Okay, I gave the method a name that is specific to Unix domain sockets 
> because this UTF-8 strangeness is not likely to
> be useful by other components.
>> One other comment on the changes to the file system provider it should be 
>> okay to change UnixUserPrinipals ad its
>> fromUid and fromGid method to be public. This would mean that the patch 
>> shouldn't need to add UnixUserGroup (the main
>> issue is that class is that it means we end up with two classes with static 
>> factory methods doing the same thing).
> 
> Okay, that does simplify it a bit.
> 
> Thanks,
> Michael.
> 
>> -Alan.
>>
>>
>>
>>
>>
>>

Michael McMahon has updated the pull request incrementally with one additional 
commit since the last revision:

  unixdomainchannels: updates after comments sent by Alan 14 Sept

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/52/files
  - new: https://git.openjdk.java.net/jdk/pull/52/files/22f37a82..36695095

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=52=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=03-04

  Stats: 1344 lines in 17 files changed: 604 ins; 704 del; 36 mod
  Patch: https://git.openjdk.java.net/jdk/pull/52.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52

PR: https://git.openjdk.java.net/jdk/pull/52

Re: RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests

[Alexey Semenyuk]

On Sat, 19 Sep 2020 02:42:27 GMT, Alexander Matveev  
wrote:

> https://bugs.openjdk.java.net/browse/JDK-8231591
> 
> - Added MultiLauncherTwoPhaseTest which uses predefine app image with 
> multiple launcher and tests to make sure installer
>   will create shortcuts for all launchers.
> - Fixed Linux DesktopIntegration to create shortcuts for additional launcher 
> if we using pre-define app image.

How about testing of other jpackage command line options in two phase mode? 
Like "--name", "--version"? Any plans to
add them?

src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/DesktopIntegration.java
 line 549:

> 547: private final List associations;
> 548:
> 549: private static boolean initAppImageLaunchers = true;

What is the logic behind static variable?

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java line 140:

> 138: cmd.verifyIsOfType(PackageType.WINDOWS);
> 139: this.cmd = cmd;
> 140: this.name = name;

I'd suggest to have the following initialization of "name" field:
`this.name = (name == null ? cmd.name() : name)`
This will help to avoid multiple `(name == null ? cmd.name() : name)` in the 
code.

test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 54:

> 52: public class MultiLauncherTwoPhaseTest {
> 53:
> 54: public static void main(String[] args) throws Exception {

Please replace main() function with dedicated test function. You can use 
SimplePackageTest as a template, This would
give better control on how to run the test in debug mode.

test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 94:

> 92: launcher2.verify(cmd);
> 93: launcher2.verifyPackageInstalled(cmd);
> 94: });

Looks like a duplicate of verify logic added with the preceding 
addInstallVerifier() call.

test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 80:

> 78: })
> 79: .addInstallVerifier(cmd -> {
> 80: launcher1.verify(cmd);

There is no need in explicit call to AdditionalLauncher.verify() as this 
function is scheduled for the call from
AdditionalLauncher.applyTo() call.  Thus there is no need to change access to 
this function from "private" to "public"
in your patch to AdditionalLauncher.java.

test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 56:

> 54: public static void main(String[] args) throws Exception {
> 55: TKit.run(args, () -> {
> 56: Path appimageOutput = TKit.workDir().resolve("appimage");

Please use TKit.createTempDirectory() to create directories for intermediate 
data of test runs. This allows clean
multiple runs of the test in the same work directory.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AdditionalLauncher.java line 
271:

> 269: } else {
> 270: TKit.assertPathExists(appInstallDir, false);
> 271: }

I don't understand what is going on here and why branching is required. Also 
seems like verifyPackageUninstalled()
function is never called.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java line 559:

> 557: && !cmd.isPackageUnpacked(
> 558: "Not verifying desktop integration")) {
> 559: new WindowsHelper.DesktopIntegrationVerifier(cmd, 
> null);

I'd rather add a loop calling DesktopIntegrationVerifier() for all launchers 
than copy/paste the code in
AdditionalLauncher.java

-

Changes requested by asemenyuk (Committer).

PR: https://git.openjdk.java.net/jdk/pull/263

Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v9]

[Ian Graves]

On Sat, 19 Sep 2020 16:04:36 GMT, Alan Bateman  wrote:

>> Ian Graves has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Using File.walk for copy
>
> test/jdk/tools/jlink/JLinkReproducible3Test.java line 112:
> 
>> 110: }
>> 111: }
>> 112:
> 
> The update to ImageFileCreator looks good.
> 
> A few nits in JLinkReproducible3Test:
> 1. Invoking copyJDKs with two destination locations looks a bit strange. I 
> think would be easier for future maintainers
> if it does one copy and is called twice. 2. Files.copy will create a 
> directory, no need for Files.createDirectories.
> 3. Are you sure that bin/jlink is executable, I just wonder if the test runs 
> with the patch have ended up depending the
> umask at the time. It is possible to specify COPY_ATTRIBUTES to the copy 
> method and it will attempt to copy the
> permissions. 4. The variable naming is more like C style, so a bit 
> inconsistent with the naming usually used in Java
> programs.

1. I agree that it's awkward and I'm not a huge fan. I'm okay doing it this 
way, but this is recreating the reproducer
for https://bugs.openjdk.java.net/browse/JDK-8252730 which is two identical 
JDK's in different directories. I can
document this in the test further or we can change it.

2-4: Noted. Will clean these up and loop back. Thanks!

-

PR: https://git.openjdk.java.net/jdk/pull/156

Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v2]

[Weijun Wang]

On Sun, 20 Sep 2020 14:14:51 GMT, Roger Riggs  wrote:

>> # JDK-8252523: Add ASN.1 Formatter to work with test utility HexPrinter
>> 
>> Debugging functions that utilize ASN.1, DER, and BER encoded streams is
>> difficult without test utilities to show the contents.
>> The ASN.1 formatter reads a stream and produces annotated output of the
>> tags, values, and structures.
>> When used with the test library jdk.test.lib.hexdump.HexPrinter the 
>> annotations are synchronized
>> with the hex formatted output.
>> 
>> Small changes to HexPrinter are included to improve the output readability.
>> 
>> 
>> Example decoding of a .pem certificate:
>> SEQUENCE [910]
>>   SEQUENCE [630]
>> CONTEXT cons 0 [3]
>>   BYTE 2,
>> BYTE 3,
>> SEQUENCE [13]
>>   OBJECT ID  [9] 1.2.840.113549.1.1.11 (SHA256withRSA)
>>   NULL
>> SEQUENCE [76]
>>   SET [11]
>> SEQUENCE [9]
>>   OBJECT ID  [3] 2.5.4.6 (CountryName)
>>   'IN'
>>   ...
>>   SET [16]
>> SEQUENCE [14]
>>   OBJECT ID  [3] 2.5.4.3 (CommonName)
>>   Client1
>> SEQUENCE [30]
>>   UTCTIME  [13] '150526221718Z'
>>   UTCTIME  [13] '250523221718Z'
>> ...
>> SEQUENCE [290]
>>   SEQUENCE [13]
>> OBJECT ID  [9] 1.2.840.113549.1.1.1 (RSA)
>> NULL
>>   BIT STRING  [271]
>>   CONTEXT cons 3 [123]
>> SEQUENCE [121]
>>   SEQUENCE [9]
>> OBJECT ID  [3] 2.5.29.19 (BasicConstraints)
>> OCTET STRING  [2] 
>>   SEQUENCE [44]
>> OBJECT ID  [9] 2.16.840.1.113730.1.13
>> OCTET STRING  [31] '..OpenSSL Generated Certificate'
>>   SEQUENCE [29]
>> OBJECT ID  [3] 2.5.29.14 (SubjectKeyID)
>> OCTET STRING  [22] 
>>   SEQUENCE [31]
>> OBJECT ID  [3] 2.5.29.35 (AuthorityKeyID)
>> OCTET STRING  [24] 
>>   SEQUENCE [13]
>> OBJECT ID  [9] 1.2.840.113549.1.1.11 (SHA256withRSA)
>> NULL
>>   BIT STRING  [257]
>> When used with the HexPrinter test utility, the formatting of the
>> hexadecimal values is selected with the parameters to HexPrinter.
>> 
>> : 30 82 03 8e ; SEQUENCE [910]
>> 0004: 30 82 02 76 ;   SEQUENCE [630]
>> 0008: a0 03   ; CONTEXT cons 
>> 0 [3]
>> 000a:   02 01 02  ;   BYTE 2,
>> 000d:02 01 03 ; BYTE 3,
>> 0010: 30 0d   ; SEQUENCE [13]
>> 0012:   06 09 2a 86 48 86 f7 0d 01 01 0b  ;   OBJECT ID  
>> [9] 1.2.840.113549.1.1.11 (SHA256withRSA)
>> 001d:05 00;   NULL
>> 001f:  30 ; SEQUENCE [76]
>> 0020: 4c  ;
>> 0021:31 0b;   SET [11]
>> 0023:  30 09  ; SEQUENCE 
>> [9]
>> 0025:06 03 55 04 06   ;   OBJECT 
>> ID  [3] 2.5.4.6 (CountryName)
>> 002a:   13 02 49 4e   ;   'IN'
>> 
>> ...   ...
>> 
>> 005b:  31 10  ;   SET [16]
>> 005d:30 0e; SEQUENCE 
>> [14]
>> 005f:  06 ;   OBJECT 
>> ID  [3] 2.5.4.3 (CommonName)
>> 0060: 03 55 04 03 ;
>> 0064: 0c 07 43 6c 69 65 6e 74 31  ;   Client1
>> 006d:30 1e; SEQUENCE [30]
>> 006f:  17 ;   UTCTIME  
>> [13] '150526221718Z'
>> 0070: 0d 31 35 30 35 32 36 32 32 31 37 31 38 5a   ;
>> 007e:   17 0d ;   UTCTIME  
>> [13] '250523221718Z'
>> 0080: 32 35 30 35 32 33 32 32 31 37 31 38 5a  ;
>> 
>> ... ...
>> 
>> 00db:  30 82 01 22; SEQUENCE 
>> [290]
>> 00df:  30 ;   SEQUENCE 
>> [13]
>> 00e0: 0d  ;
>> 00e1:06 09 2a 86 48 86 f7 0d 01 01 01 ; OBJECT 
>> ID  [9] 1.2.840.113549.1.1.1 (RSA)
>> 00ec: 05 00   ; NULL
>> 00ee:   03 82 ;   BIT STRING 
>>  [271]
>> 00f0: 01 0f 00 30 82 01 0a 02 82 01 01 00 d8 70 03 54 ;
>> 
>> ...
>> 
>> 01f0: 

Re: RFR: 8253149: Building an installer from invalid app image fails on Window…

[Alexey Semenyuk]

On Sun, 20 Sep 2020 21:23:17 GMT, Andy Herrick  wrote:

> 8253149: Building an installer from invalid app image fails on Windows and 
> Linux
> When jpackage builds a package from an app-image that was not generated by 
> jpackage, the tool should give user a
> warning message, and then complete the package anyway.

test/jdk/tools/jpackage/share/AppImagePackageTest.java line 73:

> 71: final String name = "EmptyAppImagePackageTest";
> 72: final String imageName = name + (TKit.isOSX() ? ".app" : "");
> 73: Path appImageDir = TKit.workDir().resolve(imageName);

I'd suggest to use Tkit.createTempDirectory() to create temp directory. The 
function will create unique directory every
time it is called allowing to run the test multiple times and have fresh empty 
directory in every run.

test/jdk/tools/jpackage/share/AppImagePackageTest.java line 81:

> 79: Path readme = Files.createFile(libdir.resolve("README"));
> 80: try (BufferedWriter bw = Files.newBufferedWriter(readme)) {
> 81: bw.write("This is some arbitrary ext for the README 
> file\n");

All these lines can be replaced with a single call

`TKit.createTextFile(List.of("This is some arbitrary text for the README 
file"));`

This statement will add log statements, so no manual logging is required. Also 
there is no need for try/catch block

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LinuxHelper.java line 275:

> 273: actualCriticalRuntimePaths);
> 274: } else {
> 275: // AppImagePackageTest.testEmpty() will no dependencied,

Looks like a typo

src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/resources/MainResources.properties
 line 77:

> 75:
> 76: warning.no.jdk.modules.found=Warning: No JDK Modules found
> 77: warning.forign-app-image=Warning: app-image dir ({0}) not generated by 
> jpackage.

Should the id be named "foreign" instead of "forign"?

-

PR: https://git.openjdk.java.net/jdk/pull/271

Integrated: JDK-8230652: Improve verbose output

[Andy Herrick]

On Sat, 12 Sep 2020 18:30:08 GMT, Andy Herrick  wrote:

> JDK-8230652
> Extracting the commands displayed by verbose output (including commands 
> called thru ToolProvider) , to contain the the
> command, it's output, and it's return value on separate lines and formatted 
> in a way that they can be easily cut and
> pasted into a script.

This pull request has now been integrated.

Changeset: 43be5a3c
Author:Andy Herrick 
URL:   https://git.openjdk.java.net/jdk/commit/43be5a3c
Stats: 78 lines in 9 files changed: 25 ins; 42 del; 11 mod

8230652: Improve verbose output

Reviewed-by: almatvee, asemenyuk, kizune

-

PR: https://git.openjdk.java.net/jdk/pull/141

Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char) [v3]

[Jason Tatton]

> This is an implementation of the indexOf(char) intrinsic for StringLatin1 (1 
> byte encoded Strings). It is provided for
> x86 and ARM64. The implementation is greatly inspired by the indexOf(char) 
> intrinsic for StringUTF16. To incorporate it
> I had to make a small change to StringLatin1.java (refactor of functionality 
> to intrisified private method) as well as
> code for C2.  Submitted to: hotspot-compiler-dev and core-libs-dev as this 
> patch contains a change to hotspot and
> java/lang/StringLatin1.java  https://bugs.openjdk.java.net/browse/JDK-8173585
> 
> Details of testing:
> 
> I have created a jtreg test 
> “compiler/intrinsics/string/TestStringLatin1IndexOfChar” to cover this new 
> intrinsic. Note
> that, particularly for the x86 implementation of the intrinsic, the code path 
> taken is dependent upon the length of the
> input String. Hence the test has been designed to cover all these cases. In 
> summary they are:
> - A “short” string of < 16 characters.
> - A SIMD String of 16 – 31 characters.
> - A AVX2 SIMD String of 32 characters+.
> 
> Hardware used for testing:
> -
> 
> - Intel Xeon CPU E5-2680 (JVM did not recognize this as having AVX2 support) 
> • Intel i7 processor (with AVX2 support).
> - AWS Graviton 2 (ARM 64 processor).
> 
> I also ran; ‘run-test-tier1’ and ‘run-test-tier2’ for: x86_64 and aarch64.
> 
> Possible future enhancements:
> 
> For the x86 implementation there may be two further improvements we can make 
> in order to improve performance of both
> the StringUTF16 and StringLatin1 indexOf(char) intrinsics:
> 1. Make use of AVX-512 instructions.
> 2. For “short” Strings (see below), I think it may be possible to modify the 
> existing algorithm to still use SSE SIMD
> instructions instead of a loop.
> Benchmark results:
> 
> **Without** the new StringLatin1 indexOf(char) intrinsic:
> 
> | Benchmark  | Mode | Cnt | Score | Error | Units |
> | - | - |- |- |- 
> |- |
> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **26,389.129** | ± 182.581 
> | ns/op |
> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 17,885.383 | ± 435.933  | 
> ns/op |
> 
> 
> **With** the new StringLatin1 indexOf(char) intrinsic:
> 
> | Benchmark  | Mode | Cnt | Score | Error | Units |
> | - | - |- |- |- 
> |- |
> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **17,875.185** | ± 407.716  
> | ns/op |
> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 18,292.802 | ± 167.306  | 
> ns/op |
> 
> 
> The objective of the patch is to bring the performance of StringLatin1 
> indexOf(char) in line with StringUTF16
> indexOf(char) for x86 and ARM64. We can see above that this has been 
> achieved. Similar results were obtained when
> running on ARM.

Jason Tatton has updated the pull request incrementally with one additional 
commit since the last revision:

  Add missing newline to end of vmSymbols.cpp

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/71/files
  - new: https://git.openjdk.java.net/jdk/pull/71/files/b85a7fb4..c8cc441e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=71=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=71=01-02

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

PR: https://git.openjdk.java.net/jdk/pull/71

Integrated: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode

[Naoto Sato]

On Fri, 18 Sep 2020 23:26:39 GMT, Naoto Sato  wrote:

> Hi,
> 
> Please review the fix to JDK-8253321. As in the issue, uninitialized (cached) 
> hash code was incorrectly referenced in
> equals() method. Removing it will correct the problem. Also, unrelated to the 
> issue, I fixed a parameter description in
> a private method.  Naoto

This pull request has now been integrated.

Changeset: dad6edbf
Author:Naoto Sato 
URL:   https://git.openjdk.java.net/jdk/commit/dad6edbf
Stats: 52 lines in 2 files changed: 1 ins; 48 del; 3 mod

8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling 
hashCode

Reviewed-by: joehw, rriggs

-

PR: https://git.openjdk.java.net/jdk/pull/260

Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v3]

[Alan Bateman]

On Sun, 20 Sep 2020 22:22:47 GMT, Lance Andersen  wrote:

>> Hi all,
>> 
>> Please review the fix  for JDK-8252739 which addresses an issue introduced by
>> https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored 
>> the offset specified by
>> Deflater.setDictionary.  Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly 
>> as well as the java/util/zip and
>> java/util/jar JCK tests.
>
> Lance Andersen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   More  updates to testByteBufferDirect in DeflaterDictionaryTests.java

Update to Deflater.c looks good.

DeflaterDictionaryTests looks like is a shaping up to be a good test for 
setDictionary. Are there other assertions that
should be checked, e.g. setDictionary(ByteBuffer) is specified to consume all 
bytes and it would be good to check that
the position is set to the limit. Also can the 2-arg setDictionary be tests, 
also corner cases such no bytes remaining
or invoked with null.

The naming of the tests is a bit inconsistent, e.g. ByteArrayTest vs. 
testByteBufferDirect. In the naming then I would
use "Heap" instead of "Wrap", as in testHeapByteBuffer.

In passing: The tests can use try-finally to ensure that Deflater::end is 
invoked even when the test fails. String
repeat(int) could be used to avoid duplicating "Welcome to Us Open;". Missing 
space in several "if(...)" usages.

-

PR: https://git.openjdk.java.net/jdk/pull/269

Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]

[Hannes Wallnöfer]

> This pull request is identical with the RFR previously sent for the Mercurial 
> repository:
> 
> https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html
> 
> I'm copy-pasting the comments from the original RFR below.
> 
> Most of the new code is added to the Extern class where it fits in quite 
> nicely and can use the existing supporting
> code for setting up external links.
> The default behaviour is to generate links to docs.oracle.com for released 
> versions and download.java.net for
> prereleases. Platform documentation URLs can be configured using the new 
> --link-platform-properties  option to
> provide a properties file with URLs pointing to to alternative locations. See 
> the CSR linked above for more details on
> the new options.  The feature can be disabled using the --no-platform-link 
> option, generating the same output as
> previously.   One problem I had to solve was how to handle the transition 
> from prerelease versions to final releases,
> since the location of the documentation changes in this transition. For 
> obvious reasons we don’t want to make that
> switch via code change at a time shortly before release.  The way it is done 
> is that we determine if the current
> javadoc instance is a prerelease version as indicated by the Version returned 
> by BaseConfiguration#getDocletVersion(),
> and then check whether the release/source version of the current javadoc 
> execution uses the same (latest) version. This
> means that that only the latest version will ever generate prerelease links 
> (e.g. running current javadoc 16 with
> source version 15 will generate links to the final release documentation) but 
> I think this is acceptable.  Another
> issue I spent some time getting right was tests. New releases require a new 
> element-list resource*), so tests have to
> pick up new releases. On the other hand, we don’t want hundreds of tests to 
> fail when a new release is added, ideally
> there should be one test  with a clear message. Because of this, when a 
> release is encountered for which no
> element-list is available a warning is generated instead of an error, and the 
> documentation is generated without
> platform links as if running with --no-platform-link option. This allows most 
> existing tests to pass and just the new
> test to fail with a relatively clear message of what is wrong.
> *) I also thought about generating the element-list for the current release 
> at build time. It’s quite involved, and we
>  still need to maintain element-lists for older versions, so I’m not sure 
> it’s worth it.
> 
> For existing tests that check output affected by the new option I added  the 
> --no-platform-link option to disable the
> feature. Otherwise we’d have to update those tests with each new release (or 
> else freeze the tests to use some static
> release or source version, which we don’t want either).  I updated the CSR to 
> the new code. It also needs to be
> reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181
> 
> Thanks,
> Hannes

Hannes Wallnöfer has updated the pull request incrementally with three 
additional commits since the last revision:

 - Merge pull request #1 from lahodaj/JDK-8216497
   
   Automatically generate the elements-list data from the ct.sym for releases 
11+, including the current release under
   development
 - Generating current release list for javadoc; using hardcoded lists for 
versions < 11
 - Attempting to (mostly) generate the javadoc release manifests from ct.sym 
historical data.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/171/files
  - new: https://git.openjdk.java.net/jdk/pull/171/files/2aed84f8..6d659ae3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=171=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=171=00-01

  Stats: 2007 lines in 9 files changed: 308 ins; 1698 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/171.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/171/head:pull/171

PR: https://git.openjdk.java.net/jdk/pull/171

Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char) [v2]

[Volker Simonis]

On Fri, 18 Sep 2020 11:56:09 GMT, Jason Tatton 
 wrote:

>> This is an implementation of the indexOf(char) intrinsic for StringLatin1 (1 
>> byte encoded Strings). It is provided for
>> x86 and ARM64. The implementation is greatly inspired by the indexOf(char) 
>> intrinsic for StringUTF16. To incorporate it
>> I had to make a small change to StringLatin1.java (refactor of functionality 
>> to intrisified private method) as well as
>> code for C2.  Submitted to: hotspot-compiler-dev and core-libs-dev as this 
>> patch contains a change to hotspot and
>> java/lang/StringLatin1.java  https://bugs.openjdk.java.net/browse/JDK-8173585
>> 
>> Details of testing:
>> 
>> I have created a jtreg test 
>> “compiler/intrinsics/string/TestStringLatin1IndexOfChar” to cover this new 
>> intrinsic. Note
>> that, particularly for the x86 implementation of the intrinsic, the code 
>> path taken is dependent upon the length of the
>> input String. Hence the test has been designed to cover all these cases. In 
>> summary they are:
>> - A “short” string of < 16 characters.
>> - A SIMD String of 16 – 31 characters.
>> - A AVX2 SIMD String of 32 characters+.
>> 
>> Hardware used for testing:
>> -
>> 
>> - Intel Xeon CPU E5-2680 (JVM did not recognize this as having AVX2 support) 
>> • Intel i7 processor (with AVX2 support).
>> - AWS Graviton 2 (ARM 64 processor).
>> 
>> I also ran; ‘run-test-tier1’ and ‘run-test-tier2’ for: x86_64 and aarch64.
>> 
>> Possible future enhancements:
>> 
>> For the x86 implementation there may be two further improvements we can make 
>> in order to improve performance of both
>> the StringUTF16 and StringLatin1 indexOf(char) intrinsics:
>> 1. Make use of AVX-512 instructions.
>> 2. For “short” Strings (see below), I think it may be possible to modify the 
>> existing algorithm to still use SSE SIMD
>> instructions instead of a loop.
>> Benchmark results:
>> 
>> **Without** the new StringLatin1 indexOf(char) intrinsic:
>> 
>> | Benchmark  | Mode | Cnt | Score | Error | Units |
>> | - | - |- |- |- 
>> |- |
>> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **26,389.129** | ± 182.581 
>> | ns/op |
>> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 17,885.383 | ± 435.933  | 
>> ns/op |
>> 
>> 
>> **With** the new StringLatin1 indexOf(char) intrinsic:
>> 
>> | Benchmark  | Mode | Cnt | Score | Error | Units |
>> | - | - |- |- |- 
>> |- |
>> | IndexOfBenchmark.latin1_mixed_char | avgt | 5 | **17,875.185** | ± 407.716 
>>  | ns/op |
>> | IndexOfBenchmark.utf16_mixed_char | avgt | 5 | 18,292.802 | ± 167.306  | 
>> ns/op |
>> 
>> 
>> The objective of the patch is to bring the performance of StringLatin1 
>> indexOf(char) in line with StringUTF16
>> indexOf(char) for x86 and ARM64. We can see above that this has been 
>> achieved. Similar results were obtained when
>> running on ARM.
>
> Jason Tatton has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now
> contains four commits:
>  - Merge master
>  - 8173585: further whitespace changes required by jcheck
>  - JDK-8173585 - whitespace changes required by jcheck
>  - JDK-8173585

src/hotspot/share/classfile/vmSymbols.cpp line 295:

> 293:   if (symbol == NULL)  return NO_SID;
> 294:   return find_sid(symbol);
> 295: }

I think it is common sense to have a newline at the end of a file.

-

PR: https://git.openjdk.java.net/jdk/pull/71

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v2]

[Andrew Haley]

On 21/09/2020 09:18, Bernhard Urban-Forster wrote:
> Thank you Andrew. Is the goal to reduce the patch diff in 
> `assembler_aarch64.cpp`? If so, we need to get rid of the
> retry in your patch to avoid additional calls to `random`, e.g. something 
> like this (the diff for the generated part
> would look indeed nicer with that:  
> https://gist.github.com/lewurm/2e7b0e00447696c75e00febb83034ba1 ):
> 
> --- a/src/hotspot/cpu/aarch64/aarch64-asmtest.py
> +++ b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
> @@ -13,6 +13,8 @@ class Register(Operand):
> 
>  def generate(self):
>  self.number = random.randint(0, 30)
> +if self.number == 18:
> +self.number = 17

Yes, better. Thanks.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8173585: Intrinsify StringLatin1.indexOf(char)

[Andrew Dinn]

On Fri, 18 Sep 2020 23:11:46 GMT, Jason Tatton 
 wrote:

>  >Can you explain where this restricted effect is documented?

> Certainly! I’ve found that determining the capability of the CPU and whether 
> to enable AVX2 support if the chip
> supports it is mostly controlled in: vm_version_x86.cpp specifically: 
> get_processor_features and in
> generate_get_cpu_info.

Yes, I can see what the code does. I was asking where the cpu behaviour is 
documented independent of the code.

> In order to test the patch comprehensively I had to track down an Intel Core 
> i7 (I7-9750H) processor which the
> aforementioned code permitted AVX2 instructions for (maybe this is an error 
> and it should not be enabled for this
> processor though) as most of the infrastructure I personally use here at AWS 
> runs on Intel Xeon processors - I also
> tested on a E5-2680 which the JVM does not enable AVX2 for.

'maybe'? The documentation Andrew provided mentioned Xeon E5 v3 which I believe 
is a Skylake design. However, the code
I pointed you at in vm_version_x86 which claims to detect 'early Skylake' 
designs is only disabling AVX512 support. It
still enables AVX2. Similarly, the code generates machine code to check the 
processor capabilities has a special check
if use_evex is set (i.e. AVX3 is requested) for Skylake which disables AVX512 
but does nto disable AVX2 support.

However, this is just the Intel side of things. When it comes to AMD I read 
that the AMD Zen 2 architecture, of which
the current flagship: Threadripper 3990X, is based, is able to support AVX2 
without the frequency scaling which
some/all(?) of the Intel chips incur. I personally don’t have access to one of 
these chips so I cannot confirm how it
is classified in the JVM.

Also, I found when investigating this that there is actually a JVM flag which 
can be used to control what level of AVX
is enabled: -XX:UseAVX=version.

-

PR: https://git.openjdk.java.net/jdk/pull/71

Re: 'Find' method for Iterable

[Justin Dekeyser]

I'm also thinking about something:

If `T find()` gets implemented on `Iterable` with the contract that
this method always ends, this implies that the amount of elements
available in the Iterable is finite, which makes it possible to define
a `int size()` by providing a trivial predicate.
This would in fact turn `Iterable` to a redundant interface with
`Collection`, as the only operations a collection could supply to
enrich Iterable are just the add/remove operations, which are all
optional by contract.

This would sound a bit weird. Iterable will just turn to a readonly
collection, and I think this was not the purpose (from a previous long
post there were in this newsletter, about splitting the List interface
in two interfaces, readonly and write-only).

Regards,

Justin Dekeyser

On Mon, Sep 21, 2020 at 10:31 AM Justin Dekeyser
 wrote:
>
> Hi all,
>
> Correct me if I'm wrong, but isn't the goal of Iterable be used as
> argument in a enhanced for-loop, that is: just allow a enhanced
> syntax.
> (Similarly, AUtoCloseable is what allows the try-with-resource syntax;
> Throwable is what allows the throw syntax)
>
> As such, isn't it out of scope of Iterable to implement this famous find 
> method?
> Since it's scope should just be to allow enhanced for-loop ?
>
> It's more like a general question in the Java programming language
> design actually: is one expected to code with those interfaces in a
> business-way,
> or implement them to allow a specific syntax ?
>
> Regards,
>
> Justin Dekeyser
>
> On Mon, Sep 21, 2020 at 10:08 AM Michael Kuhlmann  wrote:
> >
> > Hi Nir,
> >
> > at first I thought "Wow, it would be really cool to have that method in
> > Iterable! Why isn't it there already?"
> >
> > But after thinking about it, I'm now convinced that it would be a bad
> > idea. Because it extends the scope of this small, tiny Iterable
> > interface to something bigger which it shouldn't be.
> >
> > When some class implements Iterable, it just says "you can iterate over
> > my something which I call elements". Nothing more. Now when Iterable
> > implements find() by default, then automatically all classes which just
> > want to enable users to iterate over elements also tell them that there
> > can be something useful found in these elements, which is not
> > necessarily the case.
> >
> > For example, BitSet could immplements Iterable. That doesn't
> > make much practical sense, but from the definition of a BitSet it's
> > understandable: A BitSet can be seen as a set of integer values, why
> > shouldn't someone iterate over them. But now, when you add find() to
> > Iterable, it immediately tells users: Hey, you can find integers in me,
> > and when you found one, you get it returned. Which is beyond the use
> > case of a BitSet.
> >
> > forEach() is different, because forEach just iterates over the elements
> > and nothing more, which is in the scope of an Iterable.
> >
> > A second argument against adding find() is that such a generic method
> > could conflict with more specialized methods in subinterfaces or
> > classes. I like the idea of having indexOf(Predicate) in List
> > interface, but having both of them would be redundant.
> >
> > And a third argument is that it can break existing code. An implementor
> > of Iterable could already define a find() method, but return the index
> > of the element instead of the element itself, or throw some checked
> > exception. This code wouldn't compile any more.
> >
> > So while the idea of having find() in Iterable is great, the arguments
> > against are heavier from my point of view.
> >
> > -Michael
> >
> >
> > On 9/16/20 11:36 PM, Nir Lisker wrote:
> > > I don't see a reason to put it Collection when it extends Iterable anyway,
> > > and the method just requires iteration. As for execution time, true, it's
> > > faster, but Map uses a lot more memory, so it's a tradeoff. For smaller
> > > lists, linear time is acceptable. Currently I'm using Maps actually, but I
> > > find that when there are many small maps, having many small lists is 
> > > better
> > > for memory and the search time is similar. Additionally, a Map works only
> > > for searching by 1 key, but with a Collection/Iterable I can search by any
> > > property, and we're not about to use a Map for every property. So, 
> > > overall,
> > > I don't think Map is a competitor in this market. It's also possible to
> > > specify that the complexity is linear in an @implNote to avoid surprises.
> > >
> > > - Nir
> > >
> > > On Wed, Sep 16, 2020 at 11:59 PM Remi Forax  wrote:
> > >
> > >> - Mail original -
> > >>> De: "Nir Lisker" 
> > >>> À: "core-libs-dev" 
> > >>> Envoyé: Lundi 14 Septembre 2020 20:56:27
> > >>> Objet: 'Find' method for Iterable
> > >>
> > >>> Hi,
> > >>>
> > >>> This has probably been brought up at some point. When we need to find an
> > >>> item in a collection based on its properties, we can either do it in a
> > >>> loop, testing each item, or in a stream with filter and 

Re: 'Find' method for Iterable

[Justin Dekeyser]

Hi all,

Correct me if I'm wrong, but isn't the goal of Iterable be used as
argument in a enhanced for-loop, that is: just allow a enhanced
syntax.
(Similarly, AUtoCloseable is what allows the try-with-resource syntax;
Throwable is what allows the throw syntax)

As such, isn't it out of scope of Iterable to implement this famous find method?
Since it's scope should just be to allow enhanced for-loop ?

It's more like a general question in the Java programming language
design actually: is one expected to code with those interfaces in a
business-way,
or implement them to allow a specific syntax ?

Regards,

Justin Dekeyser

On Mon, Sep 21, 2020 at 10:08 AM Michael Kuhlmann  wrote:
>
> Hi Nir,
>
> at first I thought "Wow, it would be really cool to have that method in
> Iterable! Why isn't it there already?"
>
> But after thinking about it, I'm now convinced that it would be a bad
> idea. Because it extends the scope of this small, tiny Iterable
> interface to something bigger which it shouldn't be.
>
> When some class implements Iterable, it just says "you can iterate over
> my something which I call elements". Nothing more. Now when Iterable
> implements find() by default, then automatically all classes which just
> want to enable users to iterate over elements also tell them that there
> can be something useful found in these elements, which is not
> necessarily the case.
>
> For example, BitSet could immplements Iterable. That doesn't
> make much practical sense, but from the definition of a BitSet it's
> understandable: A BitSet can be seen as a set of integer values, why
> shouldn't someone iterate over them. But now, when you add find() to
> Iterable, it immediately tells users: Hey, you can find integers in me,
> and when you found one, you get it returned. Which is beyond the use
> case of a BitSet.
>
> forEach() is different, because forEach just iterates over the elements
> and nothing more, which is in the scope of an Iterable.
>
> A second argument against adding find() is that such a generic method
> could conflict with more specialized methods in subinterfaces or
> classes. I like the idea of having indexOf(Predicate) in List
> interface, but having both of them would be redundant.
>
> And a third argument is that it can break existing code. An implementor
> of Iterable could already define a find() method, but return the index
> of the element instead of the element itself, or throw some checked
> exception. This code wouldn't compile any more.
>
> So while the idea of having find() in Iterable is great, the arguments
> against are heavier from my point of view.
>
> -Michael
>
>
> On 9/16/20 11:36 PM, Nir Lisker wrote:
> > I don't see a reason to put it Collection when it extends Iterable anyway,
> > and the method just requires iteration. As for execution time, true, it's
> > faster, but Map uses a lot more memory, so it's a tradeoff. For smaller
> > lists, linear time is acceptable. Currently I'm using Maps actually, but I
> > find that when there are many small maps, having many small lists is better
> > for memory and the search time is similar. Additionally, a Map works only
> > for searching by 1 key, but with a Collection/Iterable I can search by any
> > property, and we're not about to use a Map for every property. So, overall,
> > I don't think Map is a competitor in this market. It's also possible to
> > specify that the complexity is linear in an @implNote to avoid surprises.
> >
> > - Nir
> >
> > On Wed, Sep 16, 2020 at 11:59 PM Remi Forax  wrote:
> >
> >> - Mail original -
> >>> De: "Nir Lisker" 
> >>> À: "core-libs-dev" 
> >>> Envoyé: Lundi 14 Septembre 2020 20:56:27
> >>> Objet: 'Find' method for Iterable
> >>
> >>> Hi,
> >>>
> >>> This has probably been brought up at some point. When we need to find an
> >>> item in a collection based on its properties, we can either do it in a
> >>> loop, testing each item, or in a stream with filter and findFirst/Any.
> >>>
> >>> I would think that a method in Iterable be useful, along the lines of:
> >>>
> >>> public  Optional find(Predicate condition) {
> >>> Objects.requireNonNull(condition);
> >>> for (T t : this) {
> >>>  if (condition.test(t)) {
> >>>  return Optional.of(t);
> >>> }
> >>> }
> >>> return Optional.empty();
> >>> }
> >>>
> >>> With usage:
> >>>
> >>> list.find(person -> person.id == 123456);
> >>>
> >>> There are a few issues with the method here such as t being null in
> >>> null-friendly collections and the lack of bound generic types, but this
> >>> example is just used to explain the intention.
> >>>
> >>> It will be an alternative to
> >>>
> >>> list.stream().filter(person -> person.id == 123456).findAny/First()
> >>> (depending on if the collection is ordered or not)
> >>>
> >>> which doesn't create a stream, similar to Iterable#forEach vs
> >>> Stream#forEach.
> >>>
> >>> Maybe with pattern matching this would become more appetizing.
> >>
> >> During the 

RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos

[Alexey Bakhtin]

Hi,

Plaese review JDK-8245527 fix which implements LDAP Channel Binding support for 
Java GSS/Kerberos.
Initial review is available at core-devs: 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html
This version removes "tls-unique" CB type from the list of possible channel 
binding types. The only supported type is
"tls-server-end-point"

CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311

Thank you
Alexey

-

Commit messages:
 - 8245527: LDAP Channel Binding support for Java GSS/Kerberos

Changes: https://git.openjdk.java.net/jdk/pull/278/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=278=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8245527
  Stats: 536 lines in 10 files changed: 524 ins; 0 del; 12 mod
  Patch: https://git.openjdk.java.net/jdk/pull/278.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/278/head:pull/278

PR: https://git.openjdk.java.net/jdk/pull/278

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v2]

[Bernhard Urban-Forster]

On Fri, 18 Sep 2020 18:38:34 GMT, Bernhard Urban-Forster  
wrote:

>> Our linux-aarch64 build fails with this:
>> cc: error: unrecognized command line option '-std=c++14'
>> when compiling 
>> build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch
>> 
>> I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is 
>> this something you are also experiencing,
>> and if so, how are you addressing it?
>
> Hey @erikj79, thank you so much for giving it a try!
> 
>> Our linux-aarch64 build fails with this:
>> cc: error: unrecognized command line option '-std=c++14'
>> when compiling 
>> build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch
> 
> Hmm, that's interesting. What environment is that exactly? What `configure` 
> line are you using there? We have tested on
> such a system: $ cat /etc/issue
> Ubuntu 19.10 \n \l
> $ bash configure --with-boot-jdk=/home/beurba/work/jdk-16+13 --with-jtreg
> $ make clean CONF=linux-aarch64-server-release
> $ make images JOBS=255 LOG=info CONF=linux-aarch64-server-release
> $ ./build/linux-aarch64-server-release/images/jdk/bin/java 
> -XshowSettings:properties -version 2>&1 | grep aarch64
> java.home = 
> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk
> os.arch = aarch64
> sun.boot.library.path = 
> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk/lib
> 
>> I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is 
>> this something you are also experiencing,
>> and if so, how are you addressing it?
> 
> Yes. As far as I understand, the problem is that `fixpath.exe` isn't built 
> properly when doing cross-compiling on
> Windows targets (as it hasn't been a thing so far). We use a workaround 
> internally
> https://gist.github.com/lewurm/c099a4b5fcd8a182510cbdeebcb41f77 , but a 
> proper solution is under discussion on
> build-dev: 
> https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html

> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on 
> [build-dev](mailto:build-...@openjdk.java.net):_
> 
> On 18/09/2020 11:14, Monica Beckwith wrote:
> 
> > This is a continuation of 
> > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
> 
> The diffs in assembler_aarch64.cpp are mostly spurious. Please try this.


Thank you Andrew. Is the goal to reduce the patch diff in 
`assembler_aarch64.cpp`? If so, we need to get rid of the
retry in your patch to avoid additional calls to `random`, e.g. something like 
this (the diff for the generated part
would look indeed nicer with that:  
https://gist.github.com/lewurm/2e7b0e00447696c75e00febb83034ba1 ):

--- a/src/hotspot/cpu/aarch64/aarch64-asmtest.py
+++ b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
@@ -13,6 +13,8 @@ class Register(Operand):

 def generate(self):
 self.number = random.randint(0, 30)
+if self.number == 18:
+self.number = 17
 return self

 def astr(self, prefix):
@@ -37,6 +39,8 @@ class GeneralRegisterOrZr(Register):

 def generate(self):
 self.number = random.randint(0, 31)
+if self.number == 18:
+self.number = 16
 return self

 def astr(self, prefix = ""):
@@ -54,6 +58,8 @@ class GeneralRegisterOrZr(Register):
 class GeneralRegisterOrSp(Register):
 def generate(self):
 self.number = random.randint(0, 31)
+if self.number == 18:
+self.number = 15
 return self

 def astr(self, prefix = ""):
@@ -1331,7 +1337,7 @@ generate(SpecialCases, [["ccmn",   "__ ccmn(zr, zr, 3u, 
Assembler::LE);",
 ["st1w",   "__ sve_st1w(z0, __ S, p1, Address(r0, 
7));", "st1w\t{z0.s}, p1, [x0, #7, MUL VL]"],
 ["st1b",   "__ sve_st1b(z0, __ B, p2, Address(sp, 
r1));","st1b\t{z0.b}, p2, [sp, x1]"],
 ["st1h",   "__ sve_st1h(z0, __ H, p3, Address(sp, 
r8));","st1h\t{z0.h}, p3, [sp, x8, LSL #1]"],
-["st1d",   "__ sve_st1d(z0, __ D, p4, Address(r0, 
r18));",   "st1d\t{z0.d}, p4, [x0, x18, LSL #3]"],
+["st1d",   "__ sve_st1d(z0, __ D, p4, Address(r0, 
r17));",   "st1d\t{z0.d}, p4, [x0, x17,
LSL #3]"],
 ["ldr","__ sve_ldr(z0, Address(sp));", 
  "ldr\tz0, [sp]"],
 ["ldr","__ sve_ldr(z31, Address(sp, -256));",  
  "ldr\tz31, [sp, #-256, MUL VL]"],
 ["str","__ sve_str(z8, Address(r8, 255));",
  "str\tz8, [x8, #255, MUL VL]"],

-

PR: https://git.openjdk.java.net/jdk/pull/212

Re: 'Find' method for Iterable

[Michael Kuhlmann]

Hi Nir,

at first I thought "Wow, it would be really cool to have that method in 
Iterable! Why isn't it there already?"


But after thinking about it, I'm now convinced that it would be a bad 
idea. Because it extends the scope of this small, tiny Iterable 
interface to something bigger which it shouldn't be.


When some class implements Iterable, it just says "you can iterate over 
my something which I call elements". Nothing more. Now when Iterable 
implements find() by default, then automatically all classes which just 
want to enable users to iterate over elements also tell them that there 
can be something useful found in these elements, which is not 
necessarily the case.


For example, BitSet could immplements Iterable. That doesn't 
make much practical sense, but from the definition of a BitSet it's 
understandable: A BitSet can be seen as a set of integer values, why 
shouldn't someone iterate over them. But now, when you add find() to 
Iterable, it immediately tells users: Hey, you can find integers in me, 
and when you found one, you get it returned. Which is beyond the use 
case of a BitSet.


forEach() is different, because forEach just iterates over the elements 
and nothing more, which is in the scope of an Iterable.


A second argument against adding find() is that such a generic method 
could conflict with more specialized methods in subinterfaces or 
classes. I like the idea of having indexOf(Predicate) in List 
interface, but having both of them would be redundant.


And a third argument is that it can break existing code. An implementor 
of Iterable could already define a find() method, but return the index 
of the element instead of the element itself, or throw some checked 
exception. This code wouldn't compile any more.


So while the idea of having find() in Iterable is great, the arguments 
against are heavier from my point of view.


-Michael


On 9/16/20 11:36 PM, Nir Lisker wrote:

I don't see a reason to put it Collection when it extends Iterable anyway,
and the method just requires iteration. As for execution time, true, it's
faster, but Map uses a lot more memory, so it's a tradeoff. For smaller
lists, linear time is acceptable. Currently I'm using Maps actually, but I
find that when there are many small maps, having many small lists is better
for memory and the search time is similar. Additionally, a Map works only
for searching by 1 key, but with a Collection/Iterable I can search by any
property, and we're not about to use a Map for every property. So, overall,
I don't think Map is a competitor in this market. It's also possible to
specify that the complexity is linear in an @implNote to avoid surprises.

- Nir

On Wed, Sep 16, 2020 at 11:59 PM Remi Forax  wrote:


- Mail original -

De: "Nir Lisker" 
À: "core-libs-dev" 
Envoyé: Lundi 14 Septembre 2020 20:56:27
Objet: 'Find' method for Iterable



Hi,

This has probably been brought up at some point. When we need to find an
item in a collection based on its properties, we can either do it in a
loop, testing each item, or in a stream with filter and findFirst/Any.

I would think that a method in Iterable be useful, along the lines of:

public  Optional find(Predicate condition) {
Objects.requireNonNull(condition);
for (T t : this) {
 if (condition.test(t)) {
 return Optional.of(t);
}
}
return Optional.empty();
}

With usage:

list.find(person -> person.id == 123456);

There are a few issues with the method here such as t being null in
null-friendly collections and the lack of bound generic types, but this
example is just used to explain the intention.

It will be an alternative to

list.stream().filter(person -> person.id == 123456).findAny/First()
(depending on if the collection is ordered or not)

which doesn't create a stream, similar to Iterable#forEach vs
Stream#forEach.

Maybe with pattern matching this would become more appetizing.


During the development of Java 8, we first tried to use Iterator/Iterable
instead of using a novel interface Stream.
But a Stream cleanly separate the lazy side effect free API from the
mutable one (Collection) and can be optimized better by the VM (it's a push
API instead of being a pull API).

The other question is why there is no method find() on Collection, i
believe it's because while find() is ok for any DB API, find() is dangerous
on a Collection because the execution time is linear, so people may use it
instead of using a Map.



- Nir


Rémi

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v2]

[Bernhard Urban-Forster]

On Fri, 18 Sep 2020 20:34:55 GMT, Erik Joelsson  wrote:

> I assume you need the rest of the PATH on Windows.

Doesn't look like it actually. I've reverted it, thanks for catching it.

-

PR: https://git.openjdk.java.net/jdk/pull/212

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v2]

[Monica Beckwith]

> This is a continuation of 
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>  
> Changes since then:
> * We've improved the write barrier as suggested by Andrew [1]
> * The define-guards around R18 have been changed to `R18_RESERVED`. This will 
> be enabled for Windows only for now but
>   will be required for the upcoming macOS+Aarch64 [2] port as well.
> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov 
> in our PR for now and built the
>   Windows-specific CPU feature detection on top of it.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html
> [2] https://openjdk.java.net/jeps/8251280

Monica Beckwith has refreshed the contents of this pull request, and previous 
commits have been removed. The
incremental views will show differences compared to the previous content of the 
PR. The pull request contains six new
commits since the last revision:

 - 8248787: G1: Workaround MSVC bug
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248670: Windows: Exception handling support on AArch64
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248660: AArch64: Make _clear_cache and _nop portable
   Summary: __builtin___clear_cache, etc.
   Contributed-by: mbeckwit, luhenry, burban
 - 8248659: AArch64: Extend CPU Feature detection
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248656: Add Windows AArch64 platform support code
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban
 - 8248498: Add build system support for Windows AArch64
   Reviewed-by:
   Contributed-by: mbeckwit, luhenry, burban

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/212/files
  - new: https://git.openjdk.java.net/jdk/pull/212/files/26e4af3a..5f9b0d35

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=212=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=212=00-01

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

PR: https://git.openjdk.java.net/jdk/pull/212