Hi Tagir, ----- Mail original ----- > De: "Tagir Valeev" <amae...@gmail.com> > À: "Stuart Marks" <stuart.ma...@oracle.com> > Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net> > Envoyé: Samedi 16 Juin 2018 06:01:35 > Objet: Re: RFR(s): 8060192: Add default method Collection.toArray(generator)
> 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 we can expect the VM to not allocate the array if once inlined the code is new MyClass[0].getClass() if it's not the case, i think it's a better idea to tweak the VM rather than try to change an API based on a pattern that should not be necessary. Rémi > > пт, 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 >>
