Re: RFR: 8254162: Implementation of Foreign-Memory Access API (Third Incubator)
On Mon, 2 Nov 2020 11:26:51 GMT, Maurizio Cimadamore wrote: >> I looked through the changes in this update. >> >> The shared memory segment support looks sound and the mechanism to close a >> shared memory segment is clever (albeit a bit surprising at first that it >> does global handshake to look for a frame in a scoped region. Also >> surprising that close can cause failure at both ends - it took me a while to >> see that this is pragmatic approach). >> >> The share method specifies NPE if thread == null but there is no thread >> parameter, is this a cut 'n paste error? Another one in registerCleaner >> where it should be NPE if the cleaner is null. >> >> I think the javadoc for the close method needs to be a bit clearer on the >> state of the memory segment when IllegalStateException is thrown. Will it be >> marked "not alive" when it fails? Does this mean there is a resource leak? I >> think an apiNote to explain the rational for why close is not idempotent is >> also needed, or maybe it should be re-visited so that close is a no-op when >> the memory segment is not alive. >> >> Now that MemorySegment is AutoCloseable then maybe the term "alive" should >> be replaced with "open" or "closed" and isAlive replaced with isOpen is >> isClosed. >> >> FileDescriptor can be attraction nuisance and forced reference counting >> everywhere that it is used. Is it needed? Could an isMapped method work >> instead? >> >> mapFromPath was in the second preview but I think the method name should be >> re-examined as it maps a file, the path just locates the file. Naming is >> subjectives but in this case using "map" or "mapFile" would fit beside the >> allocateNative methods. >> >> MappedMemorySegments. The force method specifies a write back guarantee but >> at the same time, the implNote in the class description suggests that the >> methods might be a no-op. You might want to adjust the wording to avoid any >> suggestion that force might be a no-op. >> >> The javadoc for copyFrom isn't changed in this update but I notice it >> specifies IndexOutOfBoundException when the source segment is larger than >> the receiver, have other exceptions been examined? >> >> I don't have any any comments on MemoryAccess except that it's not >> immediately clear why there are "Byte" methods that take a ByteOrder. Make >> sense for the multi-byte types of course. >> >> The updates the java/nio sources look okay but it would be helpful if the >> really long lines could be chopped down as it's just too hard to do >> side-by-side reviews when the lines are so long. A minor nit but the changes >> X-Buffer.java.template mess up the alignment of the parameters to >> copyMemory/copySwapMemory methods. > >> The javadoc for copyFrom isn't changed in this update but I notice it >> specifies IndexOutOfBoundException when the source segment is larger than >> the receiver, have other exceptions been examined? > > This exception is consistent with other uses of this exception throughout > this API (e.g. when writing a segment out of bounds). I assume the CSR needs to be updated so that it's in sync with the API changes in the latest round. - PR: https://git.openjdk.java.net/jdk/pull/548
Re: RFR: JDK-8253936 Replace ... with {@code ...} for java.sql [v2]
> ... is replaced with {@code ...} in java.sql classes.
> Please review and sponsor this change.
Vipin Sharma has updated the pull request incrementally with one additional
commit since the last revision:
Updated copyright year and other issues reporrted in review
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/707/files
- new: https://git.openjdk.java.net/jdk/pull/707/files/89245f36..0823d56f
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=707=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=707=00-01
Stats: 31 lines in 29 files changed: 0 ins; 0 del; 31 mod
Patch: https://git.openjdk.java.net/jdk/pull/707.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/707/head:pull/707
PR: https://git.openjdk.java.net/jdk/pull/707
Re: RFR: 8254742: InputStream::readNBytes(int) result may contain zeros not in input
On Tue, 3 Nov 2020 08:56:38 GMT, Aleksey Shipilev wrote: >> InputStream::readNBytes() invokes read(byte[],int,int) repeatedly to load >> bytes into a sequence of intermediate arrays. If an intermediate array is >> not completely filled before being added to the list of arrays the contents >> of which are eventually concatenated to produce the result, then the >> unfilled part of the intermediate array will contribute zeros to the result >> which are not actually in the input. This can occur for example if n < 8192 >> bytes are read into an intermediate array without filling it, and the next >> read() returns zero. It is proposed to detect when an intermediate array is >> only partially full, and to copy the valid range of the array into a new >> array which is instead appended to the list of component arrays. > > This makes sense to me. Looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/1024
Re: RFR: 8254742: InputStream::readNBytes(int) result may contain zeros not in input
On Tue, 3 Nov 2020 00:04:58 GMT, Brian Burkhalter wrote: > InputStream::readNBytes() invokes read(byte[],int,int) repeatedly to load > bytes into a sequence of intermediate arrays. If an intermediate array is not > completely filled before being added to the list of arrays the contents of > which are eventually concatenated to produce the result, then the unfilled > part of the intermediate array will contribute zeros to the result which are > not actually in the input. This can occur for example if n < 8192 bytes are > read into an intermediate array without filling it, and the next read() > returns zero. It is proposed to detect when an intermediate array is only > partially full, and to copy the valid range of the array into a new array > which is instead appended to the list of component arrays. Marked as reviewed by bchristi (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1024
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 18:53:23 GMT, Stuart Marks wrote:
>> Should there be a test that tests the default implementation in
>> `j.u.s.Stream`? Or maybe there is and I missed that?
>
> @dfuch wrote:
>> Should there be a test that tests the default implementation in
>> `j.u.s.Stream`? Or maybe there is and I missed that?
>
> The test method `testDefaultOps` does that. The stream test framework has a
> thing called `DefaultMethodStreams` that delegates everything except default
> methods to another Stream instance, so this should test the default
> implementation.
OK, there are rather too many forking threads here for me to try to reply to
any particular message, so I'll try to summarize things and say what I intend
to do.
Null tolerance. While part of me wants to prohibit nulls, the fact is that
Streams pass through nulls, and toList() would be much less useful if it were
to reject nulls. The affinity here is closer to Stream.toArray(), which allows
nulls, rather than Collectors.to[Unmodifiable]List.
Unmodifiability. Unlike with nulls, this is where we've been taking a strong
stand recently, where new constructs are unmodifiable ("shallowly immutable").
Consider the Java 9 unmodifiable collections, records, primitive classes, etc.
-- they're all unmodifiable. They're data-race-free and are resistant to a
whole class of bugs that arise from mutability.
Unfortunately, the combination of the above means that the returned List is
neither like an ArrayList nor like an unmodifiable list produced by List.of()
or by Collectors.toUnmodifiableList(). [Heavy sigh.] While I've been somewhat
reluctant to introduce this new thing, the alternatives of trying to reuse one
of the existing things are worse.
John and Rémi pointed out that the way I implemented this, using a subclass,
reintroduces the possibility of problems with megamorphic dispatch which we had
so carefully avoided by reducing the number of implementation classes in this
area. I agree this is a problem.
I also had an off-line discussion with John where we discussed the
serialization format, which unfortunately is somewhat coupled with this issue.
(Fortunately I think it can mostly be decoupled.) Given that we're introducing
a new thing, which is an unmodifiable-list-that-allows-nulls, this needs to be
manifested in the serialized form of these new objects. (This corresponds to
the new tag value of IMM_LIST_NULLS in the CollSer class.) The alternative is
to reuse the existing serialized form, IMM_LIST, for both of these cases,
relaxing it to allow nulls. This would be a serialization immutability problem.
Suppose I had an application that created a data structure that used lists from
List.of(), and I had a global assertion over that structure that it contained
no nulls. Further suppose that I serialized and deserizalized this structure.
I'd want that assertion to be preserved after deserialization. If another app
(or a future version of this app) created the structure using Stream.to
List(), this would allow nulls to leak into that structure and violate that
assertion. Therefore, the result of Stream.toList() should not be
serialization-compatible with List.of() et. al. That's why there's the new
IMM_LIST_NULLS tag in the serial format.
However, the new representation in the serial format does NOT necessarily
require the implementation to be a different class. I'm going to investigate
collapsing ListN and ListNNullsAllowed back into a single class, while
preserving the separate serialized forms. This should mitigate the concerns
about megamorphic dispatch.
-
PR: https://git.openjdk.java.net/jdk/pull/1026
RFR: 8255862: Remove @SuppressWarnings from sun.misc.Unsafe
Record classes are now a standard feature in 16. @SuppressWarnings can be removed from sun.misc.Unsafe. - Commit messages: - 8255862: Remove @SuppressWarnings from sun.misc.Unsafe Changes: https://git.openjdk.java.net/jdk/pull/1049/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1049=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255862 Stats: 6 lines in 1 file changed: 0 ins; 3 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1049/head:pull/1049 PR: https://git.openjdk.java.net/jdk/pull/1049
RFR: 8255863: Clean up test/jdk/java/lang/invoke/defineHiddenClass/BasicTest.java
test/jdk/java/lang/invoke/defineHiddenClass/BasicTest.java tests a record class. This test no longer needs to be run with --enable-preview. - Commit messages: - 8255863: Clean up test/jdk/java/lang/invoke/defineHiddenClass/BasicTest.java Changes: https://git.openjdk.java.net/jdk/pull/1048/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1048=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255863 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1048.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1048/head:pull/1048 PR: https://git.openjdk.java.net/jdk/pull/1048
Re: RFR: 8180352: Add Stream.toList() method
On 11/3/20 3:10 AM, Florian Weimer wrote: I'd expect a test here that if the list contains a null element, `List::copyOf` throws `NullPointerException`. The new Stream.toList() actually permits null elements (by design) so it goes through a different path than List::copyOf. The new tests' data provider does include nulls in the input, and these should be accepted. Rejection of nulls for List::copyOf is be handled by tests in test/jdk/java/util/List/ListFactories.java s'marks
Re: RFR: 8253892: Disable misleading-indentation on clang as well as gcc
On Tue, 3 Nov 2020 22:36:34 GMT, John Paul Adrian Glaubitz wrote: >> With clang 10.0, the compiler now detects a new class of warnings. The >> `misleading-indentation` warning has previously been disabled on gcc for >> hotspot and libfdlibm. Now we need to disable it for clang as well. > > Why do we disable the warning instead of fixing the incorrect indentations? @glaubitz Good question. :) If you want to start fixing code to get rid of disabled warnings, I will not stand in your way! For example, in hotspot and gcc, we have `parentheses comment unknown-pragmas address delete-non-virtual-dtor char-subscripts array-bounds int-in-bool-context ignored-qualifiers missing-field-initializers implicit-fallthrough empty-body strict-overflow sequence-point maybe-uninitialized misleading-indentation cast-function-type shift-negative-value`. I believe many of these should be fixed and removed from the list of disabled warnings. But this bug is about disabling a warning in one compiler that we have already decided to disable in another. - PR: https://git.openjdk.java.net/jdk/pull/1044
Re: RFR: 8180352: Add Stream.toList() method
- Mail original - > De: "Martin Desruisseaux" > À: "core-libs-dev" > Envoyé: Mardi 3 Novembre 2020 23:16:49 > Objet: Re: RFR: 8180352: Add Stream.toList() method > Hello > > Le 03/11/2020 à 21:30, fo...@univ-mlv.fr a écrit : > >> You know that you can not change the implementation of >> Collectors.toList(), and you also know that if there is a method >> Stream.toList(), people will replace the calls to >> .collect(Collectors.toList()) by a call to Stream.toList() for the >> very same reason (…snip…) >> > Would they would be so numerous to do this change without verifying if > the specifications match? (on my side I do read the method javadoc). But > even if we worry about developers not reading javadoc, the argument > could go both ways: they could assume that all Stream methods accepts > null and not read that Stream.toList() specifies otherwise. yes, that why i said to Brian that he is right that stream.toList() should return a List that accept null. > > Martin Rémi
Re: RFR: 8180352: Add Stream.toList() method
Hi Aaron, > De: "Aaron Scott-Boddendijk" > À: "Remi Forax" > Cc: "Brian Goetz" , "Stuart Marks" > , "core-libs-dev" > Envoyé: Mardi 3 Novembre 2020 21:59:37 > Objet: Re: RFR: 8180352: Add Stream.toList() method > >>> But it can not be immutable too, for the same reason. > >> Nope. The spec of toList() very clearly says: no guarantees as to the type, > >> mutability, serializability, etc etc of the returned list. That doesn't > >> mean > >> that every method returning a List added to the JDK after > >> Collectors::toList > >> must similarly disavow all such guarantees. > >> (In fact, we would be entirely justified to change the behavior of > >> Collectors::toList tomorrow, to match the proposed Stream::toList; the > >> spec was > >> crafted to preserve this option. We probably won't, because there are too > >> many > >> programmers who refuse to read the specification, and have baked in the > >> assumption that it returns an ArrayList, and this would likely just be > >> picking > >> a fight for little good reason, regardless of who is right.) > >I don't understand your argument. >>You know that you can not change the implementation of Collectors.toList(), >>and >>you also know that if there is a method Stream.toList(), people will replace > >the calls to >>.collect(Collectors.toList()) by a call to Stream.toList() for the very same >>reason but you want the semantics of Stream.toList() to be different from the > >semantics of Collectors.toList(). > Surely, taking that position, List.of(...) should not have produced an > unmodifiable list since many combinations of creating Lists would not have > produced unmodifiable lists. The problem is that people will see stream.toList() as a shorcut for stream.collect(Collectors.toList()), i've not seen my student thinking that List.of() is a shortcut for creating an ArrayList with some elements, but maybe it's because we introduce ArrayList later compared to List.of(...). > In all of the teams I've worked with, through many Java version transitions > (and > 3rdparty library version transitions), dealing with the selective adoption of > new APIs is nothing new. > This case is no different, developers will need to identify the cases in which > they can accept the list as unmodifiable and replace those uses of > Collectors.toList() and Collectors.toUnmodifiableList(). We have Collectors.toList() and Collectors.toUnmodifiableList(), would it make more sense to have stream.toList() and stream.toUnmodifiableList() instead of having stream.toList() that behave half way in betwen the semantics of Collectors.toList() and Collectors.toUnmodifiableList(). > And static-analysis will be able to propose some of those replacements for us. I believe that a tool able to suggest to use Collectors.toUnmodifiableList() instead of Collectors.toList() will rely one a global interprocedural static analysis, so such analysis will be invalid once you will update one of your dependency. > Does this mean it's more complex than a text-replace, sure. Will there be > mistakes, sure. But are there benefits to the immutability of the API result? > I > suggest that's pretty hard to debate against given the risk of a mistake is an > exception rather than bad data outcomes. You want immutability/unmodifiability over the cost of people miss mis-using the API. I think it's a sin as bad as me want stream.toList() to return a null hostile List :) As Brain implies, Java is a blue collar language, we should not forget it, whatever we think about null or immutability. And I said above perhaps we should not discussed about adding a method Stream.toList() but a method Stream.toUnmodifiableList(). > Please don't blunt the tool to make it safer for children. > -- > Aaron Scott-Boddendijk Rémi > On Wed, Nov 4, 2020 at 9:30 AM < [ mailto:fo...@univ-mlv.fr | > fo...@univ-mlv.fr > ] > wrote: >> > De: "Brian Goetz" < [ mailto:brian.go...@oracle.com | >> > brian.go...@oracle.com ] > >> > À: "Remi Forax" < [ mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] > >>> Cc: "Stuart Marks" < [ mailto:sma...@openjdk.java.net | >>> sma...@openjdk.java.net >> > ] >, "core-libs-dev" >> > < [ mailto:core-libs-dev@openjdk.java.net | core-libs-dev@openjdk.java.net >> > ] > >> > Envoyé: Mardi 3 Novembre 2020 20:54:57 >> > Objet: Re: RFR: 8180352: Add Stream.toList() method >> >>> There is no value in making users remember which stream methods are >> >>> null-hostile and which are null-tolerant; this is just more accidental >> >>> complexity. >> >> I think that ship has sailed a long ago. >> >> Some collectors are null hostile, some are not. >> >> You can make a point that a Collector is technically not the Stream API >> >> per se. >> > Yes, and this is no mere "technical argument". A Collector is a collection >> > of >> > behaviors, governed by its own spec. The behaviors passed to stream >> > operations, >> > whether they be lambdas passed by the user (`.map(x -> >> >
Re: RFR: 8253892: Disable misleading-indentation on clang as well as gcc
On Tue, 3 Nov 2020 22:25:08 GMT, Magnus Ihse Bursie wrote: > With clang 10.0, the compiler now detects a new class of warnings. The > `misleading-indentation` warning has previously been disabled on gcc for > hotspot and libfdlibm. Now we need to disable it for clang as well. Why do we disable the warning instead of fixing the incorrect indentations? - PR: https://git.openjdk.java.net/jdk/pull/1044
RFR: 8253892: Disable misleading-indentation on clang as well as gcc
With clang 10.0, the compiler now detects a new class of warnings. The `misleading-indentation` warning has previously been disabled on gcc for hotspot and libfdlibm. Now we need to disable it for clang as well. - Commit messages: - 8253892: Disable misleading-indentation on clang as well as gcc Changes: https://git.openjdk.java.net/jdk/pull/1044/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1044=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253892 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1044.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1044/head:pull/1044 PR: https://git.openjdk.java.net/jdk/pull/1044
Re: RFR: 8180352: Add Stream.toList() method
Hello Le 03/11/2020 à 21:30, fo...@univ-mlv.fr a écrit : You know that you can not change the implementation of Collectors.toList(), and you also know that if there is a method Stream.toList(), people will replace the calls to .collect(Collectors.toList()) by a call to Stream.toList() for the very same reason (…snip…) Would they would be so numerous to do this change without verifying if the specifications match? (on my side I do read the method javadoc). But even if we worry about developers not reading javadoc, the argument could go both ways: they could assume that all Stream methods accepts null and not read that Stream.toList() specifies otherwise. Martin
Re: RFR: 8180352: Add Stream.toList() method
>>> But it can not be immutable too, for the same reason. >> Nope. The spec of toList() very clearly says: no guarantees as to the type, >> mutability, serializability, etc etc of the returned list. That doesn't mean >> that every method returning a List added to the JDK after Collectors::toList >> must similarly disavow all such guarantees. >> (In fact, we would be entirely justified to change the behavior of >> Collectors::toList tomorrow, to match the proposed Stream::toList; the spec was >> crafted to preserve this option. We probably won't, because there are too many >> programmers who refuse to read the specification, and have baked in the >> assumption that it returns an ArrayList, and this would likely just be picking >> a fight for little good reason, regardless of who is right.) >I don't understand your argument. >You know that you can not change the implementation of Collectors.toList(), and you also know that if there is a method Stream.toList(), people will replace the calls to >.collect(Collectors.toList()) by a call to Stream.toList() for the very same reason but you want the semantics of Stream.toList() to be different from the semantics of Collectors.toList(). Surely, taking that position, List.of(...) should not have produced an unmodifiable list since many combinations of creating Lists would not have produced unmodifiable lists. In all of the teams I've worked with, through many Java version transitions (and 3rdparty library version transitions), dealing with the selective adoption of new APIs is nothing new. This case is no different, developers will need to identify the cases in which they can accept the list as unmodifiable and replace those uses of Collectors.toList() and Collectors.toUnmodifiableList(). And static-analysis will be able to propose some of those replacements for us. Does this mean it's more complex than a text-replace, sure. Will there be mistakes, sure. But are there benefits to the immutability of the API result? I suggest that's pretty hard to debate against given the risk of a mistake is an exception rather than bad data outcomes. Please don't blunt the tool to make it safer for children. -- Aaron Scott-Boddendijk On Wed, Nov 4, 2020 at 9:30 AM wrote: > > De: "Brian Goetz" > > À: "Remi Forax" > > Cc: "Stuart Marks" , "core-libs-dev" > > > > Envoyé: Mardi 3 Novembre 2020 20:54:57 > > Objet: Re: RFR: 8180352: Add Stream.toList() method > > >>> There is no value in making users remember which stream methods are > >>> null-hostile and which are null-tolerant; this is just more accidental > >>> complexity. > > >> I think that ship has sailed a long ago. > >> Some collectors are null hostile, some are not. > >> You can make a point that a Collector is technically not the Stream API > per se. > > > Yes, and this is no mere "technical argument". A Collector is a > collection of > > behaviors, governed by its own spec. The behaviors passed to stream > operations, > > whether they be lambdas passed by the user (`.map(x -> > x.hopeXIsntNull())`) or > > collector objects or comparators, are under the control of the user, and > it is > > the user's responsibility to pass behaviors which are compatible with > the data > > domain -- which the user knows and the streams implementation cannot > know. > > >> Because of that, i don't think we even have the choice of the semantics > for > >> Stream.toList(), it has to be the same as > stream.collect(Collectors.toList()). > > > This doesn't remotely follow. (And, if it did, I would likely not even > support > > this RFE.) > It seems you had an issue when replying to my mail, there is a paragraph > before "Because" > > > Let's take a step back, > > if we introduce a method toList() on Stream it will be used a lot, i > mean really > > a lot, to the point where people will change the code from > > stream.collect(Collectors.toList()) to use stream.toList() instead. > > > The spec of Collectors::toList was crafted to disavow pretty much > anything other > > than List-hood, in part in anticipation of this addition. > > >> But it can not be immutable too, for the same reason. > > > Nope. The spec of toList() very clearly says: no guarantees as to the > type, > > mutability, serializability, etc etc of the returned list. That doesn't > mean > > that every method returning a List added to the JDK after > Collectors::toList > > must similarly disavow all such guarantees. > > > (In fact, we would be entirely justified to change the behavior of > > Collectors::toList tomorrow, to match the proposed Stream::toList; the > spec was > > crafted to preserve this option. We probably won't, because there are > too many > > programmers who refuse to read the specification, and have baked in the > > assumption that it returns an ArrayList, and this would likely just be > picking > > a fight for little good reason, regardless of who is right.) > I don't understand your argument. > You know that you can not change the implementation of >
Re: RFR: 8180352: Add Stream.toList() method
> De: "Brian Goetz" > À: "Remi Forax" > Cc: "Stuart Marks" , "core-libs-dev" > > Envoyé: Mardi 3 Novembre 2020 20:54:57 > Objet: Re: RFR: 8180352: Add Stream.toList() method >>> There is no value in making users remember which stream methods are >>> null-hostile and which are null-tolerant; this is just more accidental >>> complexity. >> I think that ship has sailed a long ago. >> Some collectors are null hostile, some are not. >> You can make a point that a Collector is technically not the Stream API per >> se. > Yes, and this is no mere "technical argument". A Collector is a collection of > behaviors, governed by its own spec. The behaviors passed to stream > operations, > whether they be lambdas passed by the user (`.map(x -> x.hopeXIsntNull())`) or > collector objects or comparators, are under the control of the user, and it is > the user's responsibility to pass behaviors which are compatible with the data > domain -- which the user knows and the streams implementation cannot know. >> Because of that, i don't think we even have the choice of the semantics for >> Stream.toList(), it has to be the same as >> stream.collect(Collectors.toList()). > This doesn't remotely follow. (And, if it did, I would likely not even support > this RFE.) It seems you had an issue when replying to my mail, there is a paragraph before "Because" > Let's take a step back, > if we introduce a method toList() on Stream it will be used a lot, i mean > really > a lot, to the point where people will change the code from > stream.collect(Collectors.toList()) to use stream.toList() instead. > The spec of Collectors::toList was crafted to disavow pretty much anything > other > than List-hood, in part in anticipation of this addition. >> But it can not be immutable too, for the same reason. > Nope. The spec of toList() very clearly says: no guarantees as to the type, > mutability, serializability, etc etc of the returned list. That doesn't mean > that every method returning a List added to the JDK after Collectors::toList > must similarly disavow all such guarantees. > (In fact, we would be entirely justified to change the behavior of > Collectors::toList tomorrow, to match the proposed Stream::toList; the spec > was > crafted to preserve this option. We probably won't, because there are too many > programmers who refuse to read the specification, and have baked in the > assumption that it returns an ArrayList, and this would likely just be picking > a fight for little good reason, regardless of who is right.) I don't understand your argument. You know that you can not change the implementation of Collectors.toList(), and you also know that if there is a method Stream.toList(), people will replace the calls to .collect(Collectors.toList()) by a call to Stream.toList() for the very same reason but you want the semantics of Stream.toList() to be different from the semantics of Collectors.toList(). Rémi
Re: RFR: 8180352: Add Stream.toList() method
There is no value in making users remember which stream methods are null-hostile and which are null-tolerant; this is just more accidental complexity. I think that ship has sailed a long ago. Some collectors are null hostile, some are not. You can make a point that a Collector is technically not the Stream API per se. Yes, and this is no mere "technical argument". A Collector is a collection of behaviors, governed by its own spec. The behaviors passed to stream operations, whether they be lambdas passed by the user (`.map(x -> x.hopeXIsntNull())`) or collector objects or comparators, are under the control of the user, and it is the user's responsibility to pass behaviors which are compatible with the data domain -- which the user knows and the streams implementation cannot know. Because of that, i don't think we even have the choice of the semantics for Stream.toList(), it has to be the same as stream.collect(Collectors.toList()). This doesn't remotely follow. (And, if it did, I would likely not even support this RFE.) The spec of Collectors::toList was crafted to disavow pretty much anything other than List-hood, in part in anticipation of this addition. But it can not be immutable too, for the same reason. Nope. The spec of toList() very clearly says: no guarantees as to the type, mutability, serializability, etc etc of the returned list. That doesn't mean that every method returning a List added to the JDK after Collectors::toList must similarly disavow all such guarantees. (In fact, we would be entirely justified to change the behavior of Collectors::toList tomorrow, to match the proposed Stream::toList; the spec was crafted to preserve this option. We probably won't, because there are too many programmers who refuse to read the specification, and have baked in the assumption that it returns an ArrayList, and this would likely just be picking a fight for little good reason, regardless of who is right.) Please, can we stop having this same tired "I hate nulls, let's find a new way to punish people who like them" discussion for every single feature?
Integrated: 8255380: (zipfs) ZipFileSystem::readExtra can fail if zipinfo-time is not set to false
On Sun, 1 Nov 2020 20:09:32 GMT, Lance Andersen wrote: > Hi, > > Please review the fix for JDK-8255380 which addresses an issue when the Zip > file is > 4GB. Zip FS when processing the CEN extra data does not take into > account the fact that there is no specific order to how the extra data fields > are written. Info-ZIP writes the fields in a different order than Zip FS > which presents a problem when evaluating the Info-ZIP extended timestamp and > the LOC offset is 0X therefore the LOC offset needs to be read from > the EXTID_ZIP64 extra data prior to attempting to read the LOC extra data > field. > > The fix will defer reading of the LOC extra data field, if needed until all > of the CEN extra data has been processed. > > Using jdk.nio.zipfs.ZipInfo, you can see the ordering difference of the CEND > extra data fields when using Zip FS and info-zip. > > Info-zip is included with Mac OS so the test uses ProcessBuilder to execute > zip on Mac OS and Linux. > > Mach5 tests jdk-tier1, jdk-tier2, and jdk-tier3 run cleanly. > > Best, > Lance This pull request has now been integrated. Changeset: 6606e090 Author:Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/6606e090 Stats: 311 lines in 2 files changed: 272 ins; 29 del; 10 mod 8255380: (zipfs) ZipFileSystem::readExtra can fail if zipinfo-time is not set to false Reviewed-by: redestad - PR: https://git.openjdk.java.net/jdk/pull/987
Re: RFR: 8180352: Add Stream.toList() method
- Mail original - > De: "Brian Goetz" > À: "Remi Forax" , "Stuart Marks" > Cc: "core-libs-dev" > Envoyé: Mardi 3 Novembre 2020 19:02:37 > Objet: Re: RFR: 8180352: Add Stream.toList() method >>> This change introduces a new terminal operation on Stream. This looks like a >>> convenience method for Stream.collect(Collectors.toList()) or >>> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this >>> method directly on Stream enables it to do what can't easily by done by a >>> Collector. In particular, it allows the stream to deposit results directly >>> into >>> a destination array (even in parallel) and have this array be wrapped in an >>> unmodifiable List without copying. >>> >>> Hi Stuart, >>> I'm Okay with the idea of having a method toList() on Stream but really >>> dislike >>> the proposed semantics because tit is neither stream.collect(toList()) nor >>> stream.collect(toUnmodifiableList()) but something in between. >>> >>> It's true that a Stream support nulls, we want people to be able map() with >>> a >>> method that returns null and then filter out the nulls (even if using >>> flatMap >>> for this case is usually a better idea), >>> but it doesn't mean that all methods of the Stream interface has to support >>> nulls, the original idea was more to allow nulls to flow in the stream >>> because >>> at some point they will be removed before being stored in a collection. > > Uhm ... no and no. > > It does mean that all methods of the stream interface have to support > nulls. Streams are null tolerant. Because ... > > The original idea was not "the nulls can be removed later." The > original idea was "Streams are plumbing, they pass values through > pipelines, to user-specified lambdas, into arrays, etc, and the stream > plumbing should not have an opinion on the values that are flowing > through." And this was the right choice. A stream is computation, so i agree that letting nulls to flow is the right call. So i stand corrected on that point. > > There is no value in making users remember which stream methods are > null-hostile and which are null-tolerant; this is just more accidental > complexity. I think that ship has sailed a long ago. Some collectors are null hostile, some are not. You can make a point that a Collector is technically not the Stream API per se. [...] Let's take a step back, if we introduce a method toList() on Stream it will be used a lot, i mean really a lot, to the point where people will change the code from stream.collect(Collectors.toList()) to use stream.toList() instead. Because of that, i don't think we even have the choice of the semantics for Stream.toList(), it has to be the same as stream.collect(Collectors.toList()). So you are right that toList() can not be null hostile because Collectors.toList() is not null hostile. But it can not be immutable too, for the same reason. > Stuart made the right choice here. I would say half of the right choices, null hostile: right, immutable: wrong :) And if we at some point introduce a method toImmutableList() on Stream, it will have to be null hostile. Rémi
Re: RFR: 8180352: Add Stream.toList() method
I agree with Stuart and Brian that streams should be null-tolerant, including in this case. And of course I’m delighted to see an immutable container as the output to the utility method; I’m a fan of reduced-copy, race-free, bug-resistant algorithms you get from immutability. On Nov 3, 2020, at 6:53 AM, Remi Forax wrote: > > Also, adding a third immutable list creates a problem, it means that now when > we get an immutable list it can be 3 different implementations but the VM > only uses bimorphic inlining cache, > so more callsites will fail to inline because of that. I think we have > already reduced the number of implementation of immutable map from 3 to 2 for > the very same reasons. Yes, this part concerns me, with my JIT hat on. It seems to me that the problem can be removed simply by allowing the existing ListN object to contain nulls. That’s not the same thing as allowing List.of(1,2,3, null): That factory method can reject nulls, while the privileged factory method that makes an array-backed ListN can simply sidestep the null check. And if a call to Stream::toList call returns 0, 1, or 2 items, it’s a straightforward matter to test if either is null, and then choose to use either ListN or List12. No new implementation classes, at all!
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 10:06:26 GMT, Daniel Fuchs wrote: >> This change introduces a new terminal operation on Stream. This looks like a >> convenience method for Stream.collect(Collectors.toList()) or >> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this >> method directly on Stream enables it to do what can't easily by done by a >> Collector. In particular, it allows the stream to deposit results directly >> into a destination array (even in parallel) and have this array be wrapped >> in an unmodifiable List without copying. >> >> In the past we've kept most things from the Collections Framework as >> implementations of Collector, not directly on Stream, whereas only >> fundamental things (like toArray) appear directly on Stream. This is true of >> most Collections, but it does seem that List is special. It can be a thin >> wrapper around an array; it can handle generics better than arrays; and >> unlike an array, it can be made unmodifiable (shallowly immutable); and it >> can be value-based. See John Rose's comments in the bug report: >> >> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065 >> >> This operation is null-tolerant, which matches the rest of Streams. This >> isn't specified, though; a general statement about null handling in Streams >> is probably warranted at some point. >> >> Finally, this method is indeed quite convenient (if the caller can deal with >> what this operation returns), as collecting into a List is the most common >> stream terminal operation. > > Should there be a test that tests the default implementation in > `j.u.s.Stream`? Or maybe there is and I missed that? @dfuch wrote: > Should there be a test that tests the default implementation in > `j.u.s.Stream`? Or maybe there is and I missed that? The test method `testDefaultOps` does that. The stream test framework has a thing called `DefaultMethodStreams` that delegates everything except default methods to another Stream instance, so this should test the default implementation. - PR: https://git.openjdk.java.net/jdk/pull/1026
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 11:05:21 GMT, Stephen Colebourne
wrote:
>> This change introduces a new terminal operation on Stream. This looks like a
>> convenience method for Stream.collect(Collectors.toList()) or
>> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
>> method directly on Stream enables it to do what can't easily by done by a
>> Collector. In particular, it allows the stream to deposit results directly
>> into a destination array (even in parallel) and have this array be wrapped
>> in an unmodifiable List without copying.
>>
>> In the past we've kept most things from the Collections Framework as
>> implementations of Collector, not directly on Stream, whereas only
>> fundamental things (like toArray) appear directly on Stream. This is true of
>> most Collections, but it does seem that List is special. It can be a thin
>> wrapper around an array; it can handle generics better than arrays; and
>> unlike an array, it can be made unmodifiable (shallowly immutable); and it
>> can be value-based. See John Rose's comments in the bug report:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>>
>> This operation is null-tolerant, which matches the rest of Streams. This
>> isn't specified, though; a general statement about null handling in Streams
>> is probably warranted at some point.
>>
>> Finally, this method is indeed quite convenient (if the caller can deal with
>> what this operation returns), as collecting into a List is the most common
>> stream terminal operation.
>
> src/java.base/share/classes/java/util/stream/Stream.java line 1168:
>
>> 1166: * Accumulates the elements of this stream into a {@code List}.
>> The elements in
>> 1167: * the list will be in this stream's encounter order, if one
>> exists. There are no
>> 1168: * guarantees on the implementation type, mutability,
>> serializability, or
>
> It would be useful for callers to feel more confident that they will get an
> immutable instance. In java.time.* we have wording like "This interface
> places no restrictions on the mutability of implementations, however
> immutability is strongly recommended." Could something like that work here,
> emphasising that everyone implementing this method should seek to return an
> immutable list?
Yes, good point, the "no guarantee of mutability" clashes with the later
statement about the possibility of the returned instance being value-based,
which strongly implies immutability. I'll work on tuning this up to be a
stronger statement on immutability, while retaining "no-guarantee" for
implementation type, serializability, etc. I think we do want to preserve
future implementation flexibility in those areas.
-
PR: https://git.openjdk.java.net/jdk/pull/1026
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 10:58:25 GMT, Stephen Colebourne
wrote:
>> This change introduces a new terminal operation on Stream. This looks like a
>> convenience method for Stream.collect(Collectors.toList()) or
>> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
>> method directly on Stream enables it to do what can't easily by done by a
>> Collector. In particular, it allows the stream to deposit results directly
>> into a destination array (even in parallel) and have this array be wrapped
>> in an unmodifiable List without copying.
>>
>> In the past we've kept most things from the Collections Framework as
>> implementations of Collector, not directly on Stream, whereas only
>> fundamental things (like toArray) appear directly on Stream. This is true of
>> most Collections, but it does seem that List is special. It can be a thin
>> wrapper around an array; it can handle generics better than arrays; and
>> unlike an array, it can be made unmodifiable (shallowly immutable); and it
>> can be value-based. See John Rose's comments in the bug report:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>>
>> This operation is null-tolerant, which matches the rest of Streams. This
>> isn't specified, though; a general statement about null handling in Streams
>> is probably warranted at some point.
>>
>> Finally, this method is indeed quite convenient (if the caller can deal with
>> what this operation returns), as collecting into a List is the most common
>> stream terminal operation.
>
> src/java.base/share/classes/java/util/ImmutableCollections.java line 211:
>
>> 209: }
>> 210:
>> 211: switch (input.length) { // implicit null check of elements
>
> Was a variable renamed? Should "elements" be "input"?
Yes, actually that comment belongs up above. I'll fix it, thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/1026
Re: RFR: 8247781: Day periods support [v5]
> Hi, > > Please review the changes for the subject issue. This is to enhance the > java.time package to support day periods, such as "in the morning", defined > in CLDR. It will add a new pattern character 'B' and its supporting builder > method. The motivation and its spec are in this CSR: > > https://bugs.openjdk.java.net/browse/JDK-8254629 > > Naoto Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Fixed TCK test failures, added the new pattern char description in DateTimeFormatter - Changes: - all: https://git.openjdk.java.net/jdk/pull/938/files - new: https://git.openjdk.java.net/jdk/pull/938/files/297acc94..e52fe51f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=938=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=938=03-04 Stats: 19 lines in 2 files changed: 1 ins; 0 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/938.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/938/head:pull/938 PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d [v3]
On Tue, 3 Nov 2020 15:49:09 GMT, Kiran Sidhartha Ravikumar wrote: >> Hi Guys, >> >> Please review the integration of tzdata2020d to JDK. >> >> Details regarding the change can be viewed at - >> https://mm.icann.org/pipermail/tz-announce/2020-October/62.html >> Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 >> >> TestZoneInfo310.java test failure is addressed along with it. The last rule >> affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. >> >> Regression Tests pass along with JCK. >> >> Please let me know if the changes are good to push. >> >> Thanks, >> Kiran > > Kiran Sidhartha Ravikumar has updated the pull request incrementally with one > additional commit since the last revision: > > 8255226: Fixing whitespaces Looks good. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8180352: Add Stream.toList() method
This change introduces a new terminal operation on Stream. This looks like a convenience method for Stream.collect(Collectors.toList()) or Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this method directly on Stream enables it to do what can't easily by done by a Collector. In particular, it allows the stream to deposit results directly into a destination array (even in parallel) and have this array be wrapped in an unmodifiable List without copying. Hi Stuart, I'm Okay with the idea of having a method toList() on Stream but really dislike the proposed semantics because tit is neither stream.collect(toList()) nor stream.collect(toUnmodifiableList()) but something in between. It's true that a Stream support nulls, we want people to be able map() with a method that returns null and then filter out the nulls (even if using flatMap for this case is usually a better idea), but it doesn't mean that all methods of the Stream interface has to support nulls, the original idea was more to allow nulls to flow in the stream because at some point they will be removed before being stored in a collection. Uhm ... no and no. It does mean that all methods of the stream interface have to support nulls. Streams are null tolerant. Because ... The original idea was not "the nulls can be removed later." The original idea was "Streams are plumbing, they pass values through pipelines, to user-specified lambdas, into arrays, etc, and the stream plumbing should not have an opinion on the values that are flowing through." And this was the right choice. There is no value in making users remember which stream methods are null-hostile and which are null-tolerant; this is just more accidental complexity. And the root cause of that accidental complexity is the misguided belief that we can (and should) contain the consequences of nullity by making ad-hoc restrictions about nullity. That doesn't work, and all it does is add more complexity and more ways for X to not interact with Y. I understand the hatred for nulls, but it's a fantasy to think we can contain them with ad-hoc restrictions. And the aggregate cost in complexity of all the ad-hoc decisions is pretty significant. Stuart made the right choice here.
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d [v3]
> Hi Guys, > > Please review the integration of tzdata2020d to JDK. > > Details regarding the change can be viewed at - > https://mm.icann.org/pipermail/tz-announce/2020-October/62.html > Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 > > TestZoneInfo310.java test failure is addressed along with it. The last rule > affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. > > Regression Tests pass along with JCK. > > Please let me know if the changes are good to push. > > Thanks, > Kiran Kiran Sidhartha Ravikumar has updated the pull request incrementally with one additional commit since the last revision: 8255226: Fixing whitespaces - Changes: - all: https://git.openjdk.java.net/jdk/pull/1012/files - new: https://git.openjdk.java.net/jdk/pull/1012/files/8782181c..e4a565e2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1012=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1012=01-02 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1012.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1012/head:pull/1012 PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d [v2]
On Tue, 3 Nov 2020 00:21:16 GMT, Naoto Sato wrote: >> Thanks for getting back Naoto, I believe this is a long-standing issue - >> https://bugs.openjdk.java.net/browse/JDK-8227293. >> >> Looking back at the integration of tzdata2019a/tzdata2019b, the same issue >> was addressed by updating the source code - >> https://hg.openjdk.java.net/jdk/jdk/rev/79036e5e744b#l13.1. >> >> Here is some history to the issue - >> https://mail.openjdk.java.net/pipermail/i18n-dev/2019-July/002887.html >> >> Please let me know your thoughts > > Should we then remove the hack in the ZoneInfoFile.java that excludes > Gaza/Hebron instead? I had made changes to the ZoneInfoFile.java to avoid applying the logic present to Gaza/Hebron. Regression tests executed successfully. - PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d [v2]
> Hi Guys, > > Please review the integration of tzdata2020d to JDK. > > Details regarding the change can be viewed at - > https://mm.icann.org/pipermail/tz-announce/2020-October/62.html > Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 > > TestZoneInfo310.java test failure is addressed along with it. The last rule > affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. > > Regression Tests pass along with JCK. > > Please let me know if the changes are good to push. > > Thanks, > Kiran Kiran Sidhartha Ravikumar has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8255226 - 8255226: Updating ZoneInfoFile.java logic to avoid certain zones - 8255226: (tz) Upgrade time-zone data to tzdata2020d - Changes: - all: https://git.openjdk.java.net/jdk/pull/1012/files - new: https://git.openjdk.java.net/jdk/pull/1012/files/e859af83..8782181c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1012=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1012=00-01 Stats: 2120 lines in 83 files changed: 1178 ins; 580 del; 362 mod Patch: https://git.openjdk.java.net/jdk/pull/1012.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1012/head:pull/1012 PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8180352: Add Stream.toList() method
- Mail original - > De: "Stuart Marks" > À: "core-libs-dev" > Envoyé: Mardi 3 Novembre 2020 04:18:08 > Objet: RFR: 8180352: Add Stream.toList() method > This change introduces a new terminal operation on Stream. This looks like a > convenience method for Stream.collect(Collectors.toList()) or > Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this > method directly on Stream enables it to do what can't easily by done by a > Collector. In particular, it allows the stream to deposit results directly > into > a destination array (even in parallel) and have this array be wrapped in an > unmodifiable List without copying. > > In the past we've kept most things from the Collections Framework as > implementations of Collector, not directly on Stream, whereas only fundamental > things (like toArray) appear directly on Stream. This is true of most > Collections, but it does seem that List is special. It can be a thin wrapper > around an array; it can handle generics better than arrays; and unlike an > array, it can be made unmodifiable (shallowly immutable); and it can be > value-based. See John Rose's comments in the bug report: > > https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065 > > This operation is null-tolerant, which matches the rest of Streams. This isn't > specified, though; a general statement about null handling in Streams is > probably warranted at some point. > > Finally, this method is indeed quite convenient (if the caller can deal with > what this operation returns), as collecting into a List is the most common > stream terminal operation. Hi Stuart, I'm Okay with the idea of having a method toList() on Stream but really dislike the proposed semantics because tit is neither stream.collect(toList()) nor stream.collect(toUnmodifiableList()) but something in between. It's true that a Stream support nulls, we want people to be able map() with a method that returns null and then filter out the nulls (even if using flatMap for this case is usually a better idea), but it doesn't mean that all methods of the Stream interface has to support nulls, the original idea was more to allow nulls to flow in the stream because at some point they will be removed before being stored in a collection. For me allowing in 2020 to store null in a collection is very backward, all collections since 1.6 doesn't support nulls. Also, adding a third immutable list creates a problem, it means that now when we get an immutable list it can be 3 different implementations but the VM only uses bimorphic inlining cache, so more callsites will fail to inline because of that. I think we have already reduced the number of implementation of immutable map from 3 to 2 for the very same reasons. I believe that instead of inventing a third semantics that allows to store null in a collection, we should use the semantics of stream.collect(Collectors.toUnmodifiableList()) even if it means that toList() will throw a NPE if one element is null. Rémi
Integrated: 8255374: Add a dropReturn MethodHandle combinator
On Mon, 26 Oct 2020 15:18:08 GMT, Jorn Vernee wrote: > Hi, > > This patch adds a `dropReturn` combinator to `MethodHandles` that can be used > to create a new method handle that drops the return value, from a given > method handle. Similar to the following code: > > MethodHandle target = ...; > Class targetReturnType = target.type().returnType(); > if (targetReturnType != void.class) > target = filterReturnValue(target, empty(methodType(void.class, > targetReturnType))); > // use target > > But as a short-hand. > > Thanks, > Jorn > > CSR link: https://bugs.openjdk.java.net/browse/JDK-8255398 This pull request has now been integrated. Changeset: b8d4e02c Author:Jorn Vernee URL: https://git.openjdk.java.net/jdk/commit/b8d4e02c Stats: 91 lines in 2 files changed: 91 ins; 0 del; 0 mod 8255374: Add a dropReturn MethodHandle combinator Reviewed-by: redestad - PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 01:33:32 GMT, Stuart Marks wrote:
> This change introduces a new terminal operation on Stream. This looks like a
> convenience method for Stream.collect(Collectors.toList()) or
> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
> method directly on Stream enables it to do what can't easily by done by a
> Collector. In particular, it allows the stream to deposit results directly
> into a destination array (even in parallel) and have this array be wrapped in
> an unmodifiable List without copying.
>
> In the past we've kept most things from the Collections Framework as
> implementations of Collector, not directly on Stream, whereas only
> fundamental things (like toArray) appear directly on Stream. This is true of
> most Collections, but it does seem that List is special. It can be a thin
> wrapper around an array; it can handle generics better than arrays; and
> unlike an array, it can be made unmodifiable (shallowly immutable); and it
> can be value-based. See John Rose's comments in the bug report:
>
> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>
> This operation is null-tolerant, which matches the rest of Streams. This
> isn't specified, though; a general statement about null handling in Streams
> is probably warranted at some point.
>
> Finally, this method is indeed quite convenient (if the caller can deal with
> what this operation returns), as collecting into a List is the most common
> stream terminal operation.
test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/ToListOpTest.java
line 47:
> 45: }
> 46:
> 47: private void checkUnmodifiable(List list) {
I'd expect a test here that if the list contains a null element, `List::copyOf`
throws `NullPointerException`.
-
PR: https://git.openjdk.java.net/jdk/pull/1026
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 01:33:32 GMT, Stuart Marks wrote:
> This change introduces a new terminal operation on Stream. This looks like a
> convenience method for Stream.collect(Collectors.toList()) or
> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
> method directly on Stream enables it to do what can't easily by done by a
> Collector. In particular, it allows the stream to deposit results directly
> into a destination array (even in parallel) and have this array be wrapped in
> an unmodifiable List without copying.
>
> In the past we've kept most things from the Collections Framework as
> implementations of Collector, not directly on Stream, whereas only
> fundamental things (like toArray) appear directly on Stream. This is true of
> most Collections, but it does seem that List is special. It can be a thin
> wrapper around an array; it can handle generics better than arrays; and
> unlike an array, it can be made unmodifiable (shallowly immutable); and it
> can be value-based. See John Rose's comments in the bug report:
>
> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>
> This operation is null-tolerant, which matches the rest of Streams. This
> isn't specified, though; a general statement about null handling in Streams
> is probably warranted at some point.
>
> Finally, this method is indeed quite convenient (if the caller can deal with
> what this operation returns), as collecting into a List is the most common
> stream terminal operation.
src/java.base/share/classes/java/util/ImmutableCollections.java line 211:
> 209: }
> 210:
> 211: switch (input.length) { // implicit null check of elements
Was a variable renamed? Should "elements" be "input"?
src/java.base/share/classes/java/util/stream/Stream.java line 1168:
> 1166: * Accumulates the elements of this stream into a {@code List}. The
> elements in
> 1167: * the list will be in this stream's encounter order, if one
> exists. There are no
> 1168: * guarantees on the implementation type, mutability,
> serializability, or
It would be useful for callers to feel more confident that they will get an
immutable instance. In java.time.* we have wording like "This interface places
no restrictions on the mutability of implementations, however immutability is
strongly recommended." Could something like that work here, emphasising that
everyone implementing this method should seek to return an immutable list?
-
PR: https://git.openjdk.java.net/jdk/pull/1026
Re: RFR: 8180352: Add Stream.toList() method
On Tue, 3 Nov 2020 01:33:32 GMT, Stuart Marks wrote: > This change introduces a new terminal operation on Stream. This looks like a > convenience method for Stream.collect(Collectors.toList()) or > Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this > method directly on Stream enables it to do what can't easily by done by a > Collector. In particular, it allows the stream to deposit results directly > into a destination array (even in parallel) and have this array be wrapped in > an unmodifiable List without copying. > > In the past we've kept most things from the Collections Framework as > implementations of Collector, not directly on Stream, whereas only > fundamental things (like toArray) appear directly on Stream. This is true of > most Collections, but it does seem that List is special. It can be a thin > wrapper around an array; it can handle generics better than arrays; and > unlike an array, it can be made unmodifiable (shallowly immutable); and it > can be value-based. See John Rose's comments in the bug report: > > https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065 > > This operation is null-tolerant, which matches the rest of Streams. This > isn't specified, though; a general statement about null handling in Streams > is probably warranted at some point. > > Finally, this method is indeed quite convenient (if the caller can deal with > what this operation returns), as collecting into a List is the most common > stream terminal operation. Should there be a test that tests the default implementation in `j.u.s.Stream`? Or maybe there is and I missed that? - PR: https://git.openjdk.java.net/jdk/pull/1026
Re: RFR: 8254742: InputStream::readNBytes(int) result may contain zeros not in input
On Tue, 3 Nov 2020 00:04:58 GMT, Brian Burkhalter wrote: > InputStream::readNBytes() invokes read(byte[],int,int) repeatedly to load > bytes into a sequence of intermediate arrays. If an intermediate array is not > completely filled before being added to the list of arrays the contents of > which are eventually concatenated to produce the result, then the unfilled > part of the intermediate array will contribute zeros to the result which are > not actually in the input. This can occur for example if n < 8192 bytes are > read into an intermediate array without filling it, and the next read() > returns zero. It is proposed to detect when an intermediate array is only > partially full, and to copy the valid range of the array into a new array > which is instead appended to the list of component arrays. This makes sense to me. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1024
RFR: 8255798: Remove dead headless code in CompileJavaModules.gmk
The variable BUILD_HEADLESS_ONLY is no longer set. And sun/applet does not exist anymore. - Commit messages: - 8255798: Remove dead headless code in CompileJavaModules.gmk Changes: https://git.openjdk.java.net/jdk/pull/1031/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1031=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255798 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1031.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1031/head:pull/1031 PR: https://git.openjdk.java.net/jdk/pull/1031
