Package-private members in java.util.ArrayList
Hi!
JEP 181 (Nestmates) was implemented in Java 11, however, there are still
many library classes with members that were declared package-private to
avoid synthetic bridge methods. For example, ArrayList and its friends have
the following declarations:
• transient Object[] elementData; // non-private to simplify nested class
access
• private class Itr implements Iterator {
int cursor; // index of next element to return
int lastRet = -1; // index of last element returned; -1 if no such
int expectedModCount = modCount;
// prevent creating a synthetic constructor
Itr() {}
...
final void checkForComodification() {
...
}
}
• private class ListItr extends Itr implements ListIterator {
ListItr(int index) {
super();
cursor = index;
}
...
}
• final class ArrayListSpliterator implements Spliterator {
ArrayListSpliterator(int origin, int fence, int expectedModCount) {
this.index = origin;
this.fence = fence;
this.expectedModCount = expectedModCount;
}
...
}
As far as I understand, this is no longer relevant and we can freely make
all these members private without undermining performance. This will make
the code more readable and will stop confusing readers. Can we do it?
Zheka.
Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]
On Fri, 7 May 2021 12:17:11 GMT, Vyom Tewari wrote: >> RandomAccessFile.length() method for block device return always 0 > > Vyom Tewari has updated the pull request incrementally with one additional > commit since the last revision: > > fixed assigning -1 to uint64_t I am working on it. i will provide fix ASAP. - PR: https://git.openjdk.java.net/jdk/pull/3914
Re: Proposal for new interface: TimeSource
Yes please.
I often have people ask how they should solve exactly this problem and we
have several code-bases that have their own implementations of essentially
this interface.
We've used it not only for the request-contextual time localisation but for
controlling the stepping for data-processing and simulation.
A standard interface might also mean this makes its way into 1st-class
testing framework parameterisation.
--
Aaron Scott-Boddendijk
On Fri, May 7, 2021 at 11:28 AM Stephen Colebourne
wrote:
> This is a proposal to add a new interface to java.time.*
>
> public interface TimeSource {
> public static TimeSource system() { ... }
> public abstract Instant instant();
> public default long millis() {
> return instant().toEpochMilli();
> }
> public default Clock withZone(ZoneId zoneId) {
> return Clock.of(this, zoneId);
>}
> }
>
> The existing `Clock` abstract class would be altered to implement the
> interface.
> A new static method `Clock.of(TimeSource, ZoneId)` would be added.
> No changes are needed to any other part of the API.
> (Could add `Instant.now(TimeSource)`, but it isn't entirely necessary)
>
> Callers would pass around and use `TimeSource` directly, for example
> by dependency injection.
>
>
> Why add this interface?
> I've seen various indications that there is a desire for an interface
> representing a supplier of `Instant`. Specifically this desire is
> driven by the "unnecessary" time zone that `java.time.Clock` contains.
> Put simply, if the only thing you want is an `Instant`, then `Clock`
> isn't the right API because it forces you to think about time zones.
>
> A key thing that this interface allows is the separation of the OS
> Clock from the time-zone (which should generally be linked to user
> localization). A good architecture would pass `TimeSource` around the
> system and combine it with a time-zone held in a `UserContext` to get
> a `Clock`. The current design of java.time.* does not enable that
> because the `TimeSource` concept does not exist. Developers either
> have to write their own interface, or use `Clock` and ignore the time
> zone.
>
> A `Supplier` obviously performs similar functionality, but it
> lacks discoverability and understandability. Plus, injecting
> generified interfaces tends to be painful.
>
> Downsides?
> None really, other than a new type in the JDK that probably should
> have been in Java 8.
>
>
> See this ThreeTen-Extra discussion
> - https://github.com/ThreeTen/threeten-extra/issues/150
>
> Joda-Time has a `MillisProvider` that is similar:
> -
> https://www.joda.org/joda-time/apidocs/org/joda/time/DateTimeUtils.MillisProvider.html
>
> Time4J has a `TimeSource`:
> -
> https://github.com/MenoData/Time4J/blob/master/base/src/main/java/net/time4j/base/TimeSource.java
>
> Spring has a `DateTimeContext` that separates the user localization as
> per the `UserContext` described above:
> -
> https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/format/datetime/standard/DateTimeContext.html
>
> There is a similar concept `TimeSource` in `sun.net.httpserver`
>
> There may be a case to name the interface `InstantSource`, however
> `TimeSource` is a fairly widely understood name for this concept.
>
>
> There is the potential to extend the interface with something similar
> to Kotlin's `TimeMark` that would allow access to the monotonic clock,
> suitable for measurement of durations without any leap second effects:
> - https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-time-mark/
>
> Feedback?
>
> Stephen
>
Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]
On Mon, 10 May 2021 03:48:43 GMT, Vyom Tewari wrote: >>> Could required os = linux added for >>> test/jdk/java/nio/channels/FileChannel/BlockDeviceSize.java? As it is >>> decribed only run as linux. >> >> Yes, good idea. Also maybe we can see about changing it to avoid the >> dependency on /dev/sda1. > >> Could required os = linux added for >> test/jdk/java/nio/channels/FileChannel/BlockDeviceSize.java? As it is >> decribed only run as linux. > > Sure, this is separate > issue(https://bugs.openjdk.java.net/browse/JDK-8150539). We will fix it > separately. @vyommani this has broken the build on macos as you are including a linux specific header file in non-linux code! Please backout or fix ASAP. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/3914
Re: RFR: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. [v2]
On Sun, 9 May 2021 06:26:13 GMT, Alan Bateman wrote: > Could required os = linux added for > test/jdk/java/nio/channels/FileChannel/BlockDeviceSize.java? As it is > decribed only run as linux. Sure, this is separate issue(https://bugs.openjdk.java.net/browse/JDK-8150539). We will fix it separately. - PR: https://git.openjdk.java.net/jdk/pull/3914
Integrated: 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux.
On Fri, 7 May 2021 06:16:07 GMT, Vyom Tewari wrote: > RandomAccessFile.length() method for block device return always 0 This pull request has now been integrated. Changeset: 69b96f9a Author:Vyom Tewari URL: https://git.openjdk.java.net/jdk/commit/69b96f9a1b4a959c6b86f41c2259d9e4d47c8ede Stats: 15 lines in 1 file changed: 12 ins; 2 del; 1 mod 8266610: Method RandomAccessFile#length() returns 0 for block devices on linux. Reviewed-by: alanb, bpb - PR: https://git.openjdk.java.net/jdk/pull/3914
Re: New convenience methods on Stream
>From my own limited experience, I've seen .collect(Supplier) often when
an explicitly mutable collection is needed, such as with ArrayList::new
or HashSet::new.
Even though you could in theory use Stream.toList() for the ArrayList
version, I don't think this is advisable as toList isn't guaranteed to
return an ArrayList. It may even return a different type at runtime
based on the input size for instance, that's all up to the
implementation and we should not care about it.
Given these use cases where explicitly modifiable collections are
wanted, it could be more useful to add methods such as
.toModifiableList() and .toModifiableSet() rather than a shorthand
collector.
But these methods really belong to the Collectors API.
Note that for instance for Collectors.toList "there are no guarantees
on the type, mutability, serializability, or thread-safety of the List
returned" and there exists a Collectors.toUnmodifiableList() which does
guarantee an unmodifiable list. Adding explicit .toModifiableList() and
.toModifiableSet() with their respective guarantees might be helpful as
we would create more symmetry between modifiable and unmodifiable
collectors.
Adding these methods also allows returning more specialized
implementations, but it also frees up any optimizations we might want
to do for `Collectors.toList()` because we can now offer a more
explicit choice between modifiable and unmodifiable variants.
Kind regards,
Dave Franken
On Tue, 2021-05-04 at 23:12 -0700, Stuart Marks wrote:
> Hi Don,
>
> When evaluating new APIs I always find it helpful and educational to
> look for cases
> in real code where they might be applied, and what effect the API has
> at the call
> site. The search need not be exhaustive, but it's probably sufficient
> to find a
> number of representative examples. This does take some effort,
> though. For now I'll
> take a look at some examples where your first item can be applied:
>
> >
> > default > R toCollection(Supplier
> > supplier)
> > {
> > return this.collect(Collectors.toCollection(supplier));
> > }
> >
> > Usage Examples:
> >
> > HashSet set = stream.toCollection(HashSet::new);
> > TreeSet sortedSet = stream.toCollection(TreeSet::new);
> > ArrayDeque deque = stream.toCollection(ArrayDeque::new);
>
> Since I have the JDK handy I searched it for 'toCollection('. There
> are around 60
> hits -- but note that 2/3 of these are in java.sql.rowset and refer
> to an unrelated
> API. Some are in comments, and some are the implementation of
> Collectors.toCollection itself, which leaves just 14 cases. Let's
> look at a few.
>
>
> #
> src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTaskPool.
> java:164
>
>
> List opts =
> StreamSupport.stream(options.spliterator(), false)
>
> .collect(Collectors.toCollection(ArrayList::new));
>
> Using the proposed API here would result in:
>
> List opts =
> StreamSupport.stream(options.spliterator(), false)
> .toCollection(ArrayList::new));
>
> This makes the code a little bit nicer. A static import would also
> haved helped,
> though not quite as much as the new API:
>
> List opts =
> StreamSupport.stream(options.spliterator(), false)
>
> .collect(toCollection(ArrayList::new)));
>
> I also note that after some analysis of the usage of the resulting
> List, it's never
> modified -- indeed, it's used as the key of a Map -- so this could be
> replaced with
> the recently-added Stream::toList.
>
>
> #
> src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java:3
> 81
>
>
> Set platforms =
> StreamSupport.stream(providers.spliterator(),
> false)
> .flatMap(provider -
> >
> StreamSupport.stream(provider.getSupportedPlatformNames()
>
> .spliterator(),
>
> false))
>
> .collect(Collectors.toCollection(LinkedHashSet :: new));
>
> (Sorry for line wrapping. This file has some long lines.) Again,
> using the proposal
> API would shorten things a bit, but it doesn't really make much
> difference within
> the overall context:
>
> Set platforms =
> StreamSupport.stream(providers.spliterator(),
> false)
> .flatMap(provider -
> >
> StreamSupport.stream(provider.getSupportedPlatformNames()
>
> .spliterator(),
>
> false))
>
> .toCollection(LinkedHashSet :: new));
>
>
> #
> src/java.logging/share/classes/java/util/logging/LogManager.java:2138
>
>
> final Map> loggerConfigs =
>
> allKeys.collect(Collectors.groupingBy(ConfigProperty::getLoggerName,
> TreeMap::new,
>
> Collectors.toCollection(TreeSet::new)));
>
> This
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v6]
On Thu, 6 May 2021 16:40:31 GMT, Brian Burkhalter wrote:
>> Please consider this request to override the `java.io.InputStream` methods
>> `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more
>> performant implementations. The method overrides attempt to read all
>> requested bytes into a single array of the required size rather than
>> composing the result from a sequence of smaller arrays. An example of the
>> performance improvements is as follows.
>>
>> **readAllBytes**
>> Before
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031
>> ± 7.309 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181
>> ± 0.369 ops/s
>> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860
>> ± 0.048 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298
>> ± 0.183 ops/s
>>
>> After
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473
>> ± 43.425 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781
>> ± 1.273 ops/s
>> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461
>> ± 0.084 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502
>> ± 0.003 ops/s
>>
>>
>> **readNBytes**
>>
>> `N = file_size / 2`
>>
>> Before
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500
>> ± 153.353 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164
>> ± 1.104 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167
>> ± 0.141 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291
>> ± 0.211 ops/s
>>
>>
>> After
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210
>> ± 66.045 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752
>> ± 0.951 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043
>> ± 0.102 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629
>> ± 0.035 ops/s
>
> Brian Burkhalter has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8264777: Make length and position consistent with RAF; add path to OOME
> message
src/java.base/share/classes/java/io/FileInputStream.java line 319:
> 317: }
> 318:
> 319: public byte[] readNBytes(int len) throws IOException {
readNBytes(0) is specified to return an empty array, it looks like this
implementation will throw IIOBE. Can you check this?
-
PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v5]
On Fri, 7 May 2021 03:10:32 GMT, Jason Zaugg wrote: >> If the given Path represents a file, use the overload of read defined >> in FileChannel that accepts an explicit position and avoid serializing >> reads. >> >> Note: The underlying NIO implementation is not required to implement >> FileChannel.read(ByteBuffer, long) concurrently; Windows still appears >> to lock, as it returns true for NativeDispatcher.needsPositionLock. >> >> >> On MacOS X, the enclosed benchmark improves from: >> >> >> BenchmarkMode Cnt Score Error Units >> ZipFileSystemBenchmark.read avgt 10 75.311 ? 3.301 ms/op >> >> >> To: >> >> >> BenchmarkMode Cnt Score Error Units >> ZipFileSystemBenchmark.read avgt 10 12.520 ? 0.875 ms/op > > Jason Zaugg has updated the pull request incrementally with one additional > commit since the last revision: > > [zipfs] Add missing check-failed exception to position/read test > > This appears to have been omitted when this test was added. > To avoid false error reports, the condition must deal with the > edge case of zero-length entries, for which read will return -1. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3853
Re: RFR: 8266013: Unexpected replacement character handling on stateful CharsetEncoder [v2]
On Fri, 30 Apr 2021 16:11:30 GMT, Ichiroh Takiguchi wrote: >> When an invalid character is converted by getBytes() method, the character >> is converted to replacement byte data. >> Shift code (SO/SI) may not be added into right place by EBCDIC Mix charset. >> EBCDIC Mix charset encoder is stateful encoder. >> Shift code should be added by switching character set. >> On x-IBM1364, "\u3000\uD800" should be converted to "\x0E\x40\x40\x0F\x6F", >> but "\x0E\x40\x40\x6F\x0F" >> SI is not in right place. >> >> Also ISO2022 related charsets use escape sequence to switch character set. >> But same kind of issue is there. > > Ichiroh Takiguchi has updated the pull request incrementally with one > additional commit since the last revision: > > 8266013: Unexpected replacement character handling on stateful > CharsetEncoder Gentle reminder Currently stateful CharsetEncoder (like EBCDIC Mix, ISO2022 related) cannot handle replacement characters. Please give me your suggestion or advice. - PR: https://git.openjdk.java.net/jdk/pull/3719
Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v5]
On Fri, 7 May 2021 03:10:32 GMT, Jason Zaugg wrote: >> If the given Path represents a file, use the overload of read defined >> in FileChannel that accepts an explicit position and avoid serializing >> reads. >> >> Note: The underlying NIO implementation is not required to implement >> FileChannel.read(ByteBuffer, long) concurrently; Windows still appears >> to lock, as it returns true for NativeDispatcher.needsPositionLock. >> >> >> On MacOS X, the enclosed benchmark improves from: >> >> >> BenchmarkMode Cnt Score Error Units >> ZipFileSystemBenchmark.read avgt 10 75.311 ? 3.301 ms/op >> >> >> To: >> >> >> BenchmarkMode Cnt Score Error Units >> ZipFileSystemBenchmark.read avgt 10 12.520 ? 0.875 ms/op > > Jason Zaugg has updated the pull request incrementally with one additional > commit since the last revision: > > [zipfs] Add missing check-failed exception to position/read test > > This appears to have been omitted when this test was added. > To avoid false error reports, the condition must deal with the > edge case of zero-length entries, for which read will return -1. Hi Jason, I have made a pass through your proposed changes and they look OK. I am in the process of running our various Mach5 tiers against your patch to see if any unforeseen issues arise Best Lance - PR: https://git.openjdk.java.net/jdk/pull/3853
Re: [External] : Re: ReversibleCollection proposal
When I thought about Collection's role in the hierarchy, it seemed to me that 'Collection' is an interface for describing how elements are stored in a cardboard box (we can and and remove them) and that we might need a different, yet related, interface to describe how to retrieve the items from the box. This way we are not tied to the Collection hierarchy and we could have one Set implementation which is ordered and another Set implementation which is not and they would both still implement Collection, but only one would implement the interface. Imagine an interface like 'java.lang.OrderedEnumerable` if you will with methods such as 'first(), last(), etc', then each implementation would be free to choose to implement the interface or not. I also thought about 'OrderedIterable', but there would be too much confusion with 'Iterable', but I think these are related too. Retrieving items is an iteration problem, not a storage problem. While I would love to see the Collection hierarchy redesigned to also allow for ImmutableCollection which for instance would not have an `add(T e)` method, I don't think we can simply do something like that without breaking too many things. So in short, separating retrieval aspects from storage aspects with a different interface might be the way to go. Kind regards, Dave Franken On Wed, 2021-05-05 at 12:41 +0200, fo...@univ-mlv.fr wrote: > - Mail original - > > De: "Stuart Marks" > > À: "Remi Forax" > > Cc: "core-libs-dev" > > Envoyé: Mercredi 5 Mai 2021 02:00:03 > > Objet: Re: [External] : Re: ReversibleCollection proposal > > > On 5/1/21 5:57 AM, fo...@univ-mlv.fr wrote: > > > I suppose the performance issue comes from the fact that > > > traversing a > > > LinkedHahSet is slow because it's a linked list ? > > > > > > You can replace a LinkedHashSet by a List + Set, the List keeps > > > the values in > > > order, the Set avoids duplicates. > > > > > > Using a Stream, it becomes > > > Stream.of(getItemsFromSomeplace(), getItemsFromAnotherPlace(), > > > getItemsFromSomeplaceElse()) > > > .flatMap(List::stream) > > > .distinct() // use a Set internally > > > .collect(toList()); > > > > The problem with any example is that simplifying assumptions are > > necessary for > > showing the example, but those assumptions enable it to be easily > > picked apart. > > Of > > course, the example isn't just a particular example; it is a > > *template* for a > > whole > > space of possible examples. Consider the possibility that the items > > processing > > client needs to do some intermediate processing on the first group > > of items > > before > > adding the other groups. This might not be possible to do using > > streams. Use > > your > > imagination. > > > > > I think there are maybe some scenarios where ReversibleCollection > > > can be useful, > > > but they are rare, to the point where when there is a good > > > scenario for it > > > people will not recognize it because ReversibleCollection will > > > not be part of > > > their muscle memory. > > > > I'm certain that uses of RC/RS will be rare in the beginning, > > because they will > > be > > new, and people won't be familar with them. And then there will the > > people who > > say > > "I can't use them because I'm stuck on JDK 11 / 8 / 7 / 6 " It > > was the same > > thing with lambdas and streams in Java 8, with List.of() etc in > > Java 9, records > > in > > Java 16, etc. This wasn't an argument not to add them, and it isn't > > an argument > > not > > to add RC/RS. > > All the changes you are listing here are "client side" changes, the > ones that can be adopted quickly because they do not require to > change the API side of any libraries. > ReversibleCollection is an API side change, like generics is, those > changes have a far higher cost because you have to wait your library > dependencies to be updated. > On the Valhalla list, we have discussed several times about how to > alleviate those API side change cost using automatic bridging or > methods forwarding, even for Valhalla, we are currently moving in a > state where those mechanisms are not needed. > > > > > > There is a real value to add methods like > > > descendingSet/descendingList()/getFirst/getLast on existing > > > collections, but we > > > don't need a new interface (or two) for that. > > > > It depends on what you mean by "need". Sure, we could get away > > without this; > > after > > all, we've survived the past twenty years without it, so we could > > probably > > survive > > the next twenty years as well. > > > > It would indeed be useful to add various methods to List, Deque, > > SortedSet, and > > LinkedHashSet to provide a full set of methods on all of them. And > > it would also > > be > > nice to have those methods be similar to one another. An interface > > helps with > > that, > > but I agree, that's not really the reason to have an interface > > though. > > > > The reversed-view concept could
