Re: 'Find' method for Iterable

2020-09-19 Thread Nir Lisker
>
> While it might not be difficult to add a find() method to Iterable, why
> limit it to
> the find operation, and what about all the other operations available on
> Stream?


Good question. I would say it's a matter of how much it is used and what
it takes to implement it. The find operation is a lot more common than
reduce from what I observe, for example, so I wouldn't suggest reduce to be
added..A map(Function) operation would require creating a new
collection/iterable internally, and that can be messy (you could
preemptively create and pass your own, but then I doubt the worthiness of
it). forEach already exists. I just don't see anything enticing.

Maybe what's necessary is a way to convert an Iterable to a Stream.


Most Iterables are Collections and arrays, and these are easy to convert,
so I'm not sure if that really helps. Besides,the idea is to avoid Stream,
as I've mentioned, due to the cumbersomeness and the overhead of creating a
stream. If I need to do

iterable.stream().filter(person -> person.id == 123456).findAny/First()

then I didn't really solve my problem.

On the other hand, your examples use a list. The List interface already has
> methods
> indexOf/lastIndexOf which search the list for a particular object that's
> compared
> using equals(). It seems reasonable to consider similar methods that take a
> predicate instead of an object.


I could have used a Set just as well. As for indexOf(Predicate), I would
say that it is useful (but personally, I hardly ever need the index of an
object, I need the object itself). Interestingly, removeIf(Predicate)
exists, but remove(Predicate) doesn't. I would think twice before
suggesting to add it though.

Ultimately, you have access to a lot of analytics and codebase scans. If
you know which patterns are used a lot more than others it would be a good
guide. If there are a lot of iterations in order to find an object, its
index, or to remove it (or something else), perhaps it's worth supplying
these methods. After all, forEach(Consumer) exists and it iterates while
calling accept(t) - not that different from iterating with test(t).

P.S. lastIndexOf I find odd in the sense that it's the only method I found
that iterates backwards, We don't have, removeLast, removeIfBackwards,
forEachBackwards, a backwards for-each loop, or addLast (the latter is
add(list.size()-1, e); ).

- Nir

On Thu, Sep 17, 2020 at 1:32 AM Stuart Marks 
wrote:

>
>
> On 9/16/20 1: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.
>
>
> Hi Nir,
>
> Rémi is correct to point out this distinction between the lazy operations
> (which
> appear on Stream) and the eager (and possibly mutating) operations on
> Collections. I
> think we want to preserve this distinction.
>
> While it might not be difficult to add a find() method to Iterable, why
> limit it to
> the find operation, and what about all the other operations available on
> Stream?
> Maybe what's necessary is a way to convert an Iterable to a Stream. In
> fact, this is
> already possible:
>
>  

Re: RFR: 8252537: Updated @exception with @throws [v3]

2020-09-19 Thread Vipin Sharma
> Updated @exception with @throws for core-libs, it fixes all open sub-tasks of 
> JDK-8252536.
> 
> Open Subtasks part of this fix are:
> 1. JDK-8252537
> 2. JDK-8252539
> 3. JDK-8252540
> 4. JDK-8252541
> 
> Previous conversation on this:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-September/068540.html

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

  JDK-8252537 Updated @exception with @throws, implemented review comments

-

Changes: https://git.openjdk.java.net/jdk/pull/95/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=95=02
  Stats: 1690 lines in 86 files changed: 0 ins; 0 del; 1690 mod
  Patch: https://git.openjdk.java.net/jdk/pull/95.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/95/head:pull/95

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


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

2020-09-19 Thread Alan Bateman
On Fri, 18 Sep 2020 20:40:28 GMT, Ian Graves  wrote:

>> 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:
> 
>   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.

-

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


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

2020-09-19 Thread Alan Bateman
On Fri, 18 Sep 2020 23:47:56 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

src/java.base/share/classes/jdk/internal/misc/CDS.java line 42:

> 40: public static native void defineArchivedModules(ClassLoader 
> platformLoader, ClassLoader systemLoader);
> 41:
> 42: public static native long getRandomSeedForCDSDump();

The moving of the archive methods to CDS looks okay but inconsistent to only 
comment 3 of the 5 methods.

-

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


Re: RFR: 8252537: Updated @exception with @throws [v2]

2020-09-19 Thread Vipin Sharma
On Tue, 15 Sep 2020 19:19:07 GMT, Roger Riggs  wrote:

>> @RogerRiggs I understand your point and will update PR with correct 
>> indentation.
>> But I think adding 3 spaces after throws may not be right for all cases.
>> For example when
>> 1. Another tag in same method is using only 1 space.
>> 2. In some cases (e.g. free method of Blob.java) we had a mix of throws and 
>> exception in the same method both with one
>> space after. Here after adding 3 spaces throws tags will have the different 
>> number of spaces and indentation will not
>> be same as before.  I will update PR to make sure the indentation looks same 
>> as before and there is no change in
>> javadoc. Please tell me in case my understnding is not correct here.
>
> HI Vipin,
> 
> Correct, a better description is "fix the indentation".
> I mnetioned 3 because that was the difference in length between
> "exception" and "throws".
> 
> Thanks for the followup, Roger
> 
> 
> 
> On 9/15/20 3:14 PM, Vipin Sharma wrote:
>>
>> @RogerRiggs
>> 
>> I understand your point and will update PR with correct indentation.
>> But I think adding 3 spaces after throws may not be right for all cases.
>> For example when
>>
>>  1. Another tag in same method is using only 1 space.
>>  2. In some cases (e.g. free method of Blob.java) we had a mix of
>> throws and exception in the same method both with one space after.
>> Here after adding 3 spaces throws tags will have the different
>> number of spaces and indentation will not be same as before.
>>
>> I will update PR to make sure the indentation looks same as before and
>> there is no change in javadoc.
>> Please tell me in case my understnding is not correct here.
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> ,
>> or unsubscribe
>> .
>>

There was something wrong with the way I updated this PR, please ignore this. I 
will try to correct this or will close
this PR and create a new one.

-

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


Re: RFR: 8252537: Updated @exception with @throws [v2]

2020-09-19 Thread Vipin Sharma
> Updated @exception with @throws for core-libs, it fixes all open sub-tasks of 
> JDK-8252536.
> 
> Open Subtasks part of this fix are:
> 1. JDK-8252537
> 2. JDK-8252539
> 3. JDK-8252540
> 4. JDK-8252541
> 
> Previous conversation on this:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-September/068540.html

Vipin Sharma 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:

 - JDK-8252537 Updated @exception with @throws
 - JDK-8252537 Updated @exception with @throws
 - JDK-8252537 Updated @exception with @throws

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/95/files
  - new: https://git.openjdk.java.net/jdk/pull/95/files/a84158ac..e9953466

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

  Stats: 20314 lines in 606 files changed: 10333 ins; 7230 del; 2751 mod
  Patch: https://git.openjdk.java.net/jdk/pull/95.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/95/head:pull/95

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


Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode [v2]

2020-09-19 Thread Roger Riggs
On Sat, 19 Sep 2020 01:36:38 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
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressing Joe's comments

Marked as reviewed by rriggs (Reviewer).

-

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


Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support

2020-09-19 Thread Andrew Haley
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.

-- 
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
diff --git a/src/hotspot/cpu/aarch64/aarch64-asmtest.py 
b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
index f5a5c6b5aee..43bac8e8142 100644
--- a/src/hotspot/cpu/aarch64/aarch64-asmtest.py
+++ b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
@@ -12,8 +12,11 @@ class Operand(object):
 class Register(Operand):
 
 def generate(self):
-self.number = random.randint(0, 30)
-return self
+while True:
+self.number = random.randint(0, 30)
+# r18 is used for TLS on Windows ABI.
+if self.number != 18:
+return self
 
 def astr(self, prefix):
 return prefix + str(self.number)
@@ -36,8 +39,10 @@ class GeneralRegister(Register):
 class GeneralRegisterOrZr(Register):
 
 def generate(self):
-self.number = random.randint(0, 31)
-return self
+while True:
+self.number = random.randint(0, 31)
+if self.number != 18:
+return self
 
 def astr(self, prefix = ""):
 if (self.number == 31):
@@ -53,8 +58,10 @@ class GeneralRegisterOrZr(Register):
 
 class GeneralRegisterOrSp(Register):
 def generate(self):
-self.number = random.randint(0, 31)
-return self
+while True:
+self.number = random.randint(0, 31)
+if self.number != 18:
+return self
 
 def astr(self, prefix = ""):
 if (self.number == 31):
@@ -1331,7 +1338,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]"],


Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support

2020-09-19 Thread Andrew Haley
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
>
> Changes since then:
> * We've improved the write barrier as suggested by Andrew [1]

It's still wrong, I'm afraid. This is not a full barrier:

+#define FULL_MEM_BARRIER atomic_thread_fence(std::memory_order_acq_rel);

it is only StoreStore|LoadStore|LoadLoad, but you need StoreLoad as
well. It might well be that you get the same DMB ISH instruction, but
unless you use a StoreLoad barrier here it's theoretically possible
for a compiler to reorder accesses so that a processor sees its own
stores before other processors do. (And it's confusing for the reader
too.)

Use:

+#define FULL_MEM_BARRIER atomic_thread_fence(std::memory_order_seq_cst);

See here:

https://en.cppreference.com/w/cpp/atomic/memory_order

memory_order_seq_cst "...plus a single total order exists in which all
threads observe all modifications in the same order (see
Sequentially-consistent ordering below)"

-- 
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