Hello! If you are going to create long-living array which is likely to be empty, it's good to deduplicate them to spare the heap space. This can be easily done via existing toArray method like collection.toArray(MyClass.EMPTY_ARRAY) assuming we have an empty array constant. We use this pattern very often in IntelliJ IDEA source code (e. g. think of method which returns an array of Java member annotations; often there are none). New generator methods is much less convenient for this: toArray(s -> s == 0?MyClass.EMPTY_ARRAY:new MyClass[s]). I'm actually missing a method accepting an empty array instead of generator in the Stream API. And I don't think that new collection method is useful. The existing one is perfect.
With best regards, Tagir Valeev пт, 15 июня 2018 г., 8:03 Stuart Marks <stuart.ma...@oracle.com>: > Hi all, > > I wanted to restart review of the changeset for bug JDK-8060192 [1]. The > latest > webrev is here: [2]. > > The previous review was from December 2017: [3]. The only difference > between the > current changeset and the previous one is the removal of the overriding > implementations (see explanation below). Much of the discussion in the > previous > review thread was around performance and the shape of the API. > > # Performance > > I previously had done some benchmarking on this and reported the results: > [4]. > (I've recently re-done the benchmarks and conclusions are essentially the > same.) > > The upshot is that implementations that use Arrays.copyOf() are the > fastest, > probably because it's an intrinsic. It can avoid zero-filling the freshly > allocated array because it knows the entire array contents will be > overwritten. > This is true regardless of what the public API looks like. The default > implementation calls generator.apply(0) to get a zero-length array and > then > simply calls toArray(T[]). This goes through the Arrays.copyOf() fast > path, so > it's essentially the same speed as toArray(new T[0]). > > The overrides I had previously provided in specific implementation classes > like > ArrayList actually are slower, because the allocation of the array is done > separately from filling it. This necessitates the extra zero-filling step. > Thus, > I've removed the overrides. > > # API Shape > > There was some discussion about whether it would be preferable to have a > Class > parameter as a type token for the array component type or the array type > itself, > instead of using an IntFunction generator. Most of this boils down to what > people are comfortable with. However, there are a few points that make the > generator function approach preferable. > > One point in favor of using a generator is that Stream already has a > similar > toArray(generator) method. > > Comparing this to the type token alternatives, for simple tasks like > converting > Collection<String> to String[], things are about equal: > > toArray(String[]::new) > toArray(String.class) > toArray(String[].class) > > However, things are hairier if the element type of the collection is > generic, > such as Collection<List<String>>. The result type is a generic array > List<String>[]. Using class literals or array constructor references will > necessitate using an unchecked cast, because none of these can be used to > represent a generic type. > > However, it's possible to use a method reference to provide a properly > generic > IntFunction. This would enable the toArray(generator) method to be used > without > any unchecked casts. (The method it refers to might need have an unchecked > cast > within it, though.) Such a method could also be reused for the > corresponding > Stream.toArray(generator) method. > > For these reasons I'd like to proceed with adding toArray(generator) API. > > Thanks, > > s'marks > > > [1] https://bugs.openjdk.java.net/browse/JDK-8060192 > > [2] http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.3/ > > [3] > http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050325.html > > [4] > http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050585.html > >
