Use sized array for List.toArray() #12153

izeye · 2018-02-21T08:32:34Z

This PR changes to use sized array for List.toArray().

rajadilipkolli · 2018-02-21T08:37:24Z

...nfigure/data/couchbase/CouchbaseReactiveAndImperativeRepositoriesAutoConfigurationTests.java

@@ -92,7 +92,7 @@ public void shouldCreateInstancesForReactiveAndImperativeRepositories() {
 					CouchbaseReactiveRepositoriesAutoConfiguration.class }) {
 				names.add(type.getName());
 			}
-			return names.toArray(new String[0]);
+			return names.toArray(new String[names.size()]);


Can't we write it using method reference as return names.toArray(String[]::new) ?

No, I don't think so. AFAIK, you can't create an array using that syntax. Even if you could, it would presumably give you a 0-length array which would defeat the purpose of the change that @izeye is proposing.

@wilkinsona You are correct, I mistook it

joshiste · 2018-02-21T13:10:17Z

Afaik a sized array is not always the fastest solution. When using a sized array the VM needs to initialize it with all zeroes, so it will be inited twice. When using a unsized array the VM can omit the ~~second~~ first init as it knows that all positions ~~are already empty~~ will be set. So it depends on the array size if the double init or the additional instantiation is faster...

izeye · 2018-02-21T14:18:49Z

@joshiste Thanks for the feedback! I didn't intend to gain better performance via this change but tried to align them with the other parts in codebase and simply avoid unnecessary empty array creation. By the way I'm curious on the double initialization part from your comment. Could you explain more on it as I don't understand where the double initialization is coming from?

wilkinsona · 2018-02-21T14:48:46Z

FWIW, I don't understand either. If the passed in array is too small, a new array is initialized reflectively. Once armed with an appropriate array, System.arraycopy is used to populate it. The only difference when the passed in array is used is that, if its length is greater than the list's size, array[size] is nulled out. I'd be surprised if the evaluation of the extra if condition and setting a single element to null is slower than reflectively creating a new array.

joshiste · 2018-02-21T15:30:33Z

I've just done a simple JMH on this. https://gist.github.com/joshiste/9c49661be53b752b36042b05581dbf20

Results:

(please note the logarithmic scale)

Benchmark               (size)   Mode  Cnt         Score         Error  Units
ArrayInit.doubledSize        1  thrpt   20  38500301.375 ±  477791.966  ops/s
ArrayInit.doubledSize       10  thrpt   20  29310356.815 ±  277465.983  ops/s
ArrayInit.doubledSize      100  thrpt   20   5571228.302 ±  129209.792  ops/s
ArrayInit.doubledSize     1000  thrpt   20    473345.508 ±   12074.610  ops/s
ArrayInit.doubledSize    10000  thrpt   20     46048.324 ±     837.223  ops/s
ArrayInit.doubledSize   100000  thrpt   20      2568.249 ±      48.178  ops/s
ArrayInit.doubledSize  1000000  thrpt   20       183.246 ±       0.854  ops/s
ArrayInit.halfSize           1  thrpt   20  26512697.710 ±  379465.736  ops/s
ArrayInit.halfSize          10  thrpt   20  27783698.499 ±  451057.759  ops/s
ArrayInit.halfSize         100  thrpt   20   6168023.668 ±  131382.264  ops/s
ArrayInit.halfSize        1000  thrpt   20    556418.365 ±   27003.580  ops/s
ArrayInit.halfSize       10000  thrpt   20     52891.933 ±     577.345  ops/s
ArrayInit.halfSize      100000  thrpt   20      2958.170 ±      96.353  ops/s
ArrayInit.halfSize     1000000  thrpt   20       185.678 ±       5.977  ops/s
ArrayInit.perfectFit         1  thrpt   20  37522113.175 ±  623601.483  ops/s
ArrayInit.perfectFit        10  thrpt   20  28695350.569 ± 1579416.465  ops/s
ArrayInit.perfectFit       100  thrpt   20   6009344.886 ±  350704.834  ops/s
ArrayInit.perfectFit      1000  thrpt   20    533221.149 ±   23782.529  ops/s
ArrayInit.perfectFit     10000  thrpt   20     49193.196 ±    2090.297  ops/s
ArrayInit.perfectFit    100000  thrpt   20      2759.403 ±      69.535  ops/s
ArrayInit.perfectFit   1000000  thrpt   20       191.008 ±       1.925  ops/s
ArrayInit.zeroSize           1  thrpt   20  76837570.300 ± 6328017.558  ops/s
ArrayInit.zeroSize          10  thrpt   20  46996171.285 ±  506615.172  ops/s
ArrayInit.zeroSize         100  thrpt   20   7102430.863 ±   87709.353  ops/s
ArrayInit.zeroSize        1000  thrpt   20    638110.741 ±    7825.890  ops/s
ArrayInit.zeroSize       10000  thrpt   20     56099.741 ±     449.873  ops/s
ArrayInit.zeroSize      100000  thrpt   20      2995.854 ±     140.905  ops/s
ArrayInit.zeroSize     1000000  thrpt   20       190.813 ±       1.512  ops/s

joshiste · 2018-02-21T16:00:30Z

Imho this proves (for smaller array sizes) that calling toArray(new String[0]) yields the highest throughput.

izeye · 2018-02-21T16:36:54Z

@joshiste Thanks for the feedback! I found a few articles backing your feedback like this one: https://shipilev.net/blog/2016/arrays-wisdom-ancients/

I didn't read them thoroughly yet but based on the articles the opposite direction of this PR looks right. I'll look into it soon.

wilkinsona · 2018-02-21T16:39:05Z

Very interesting (and surprising). Thanks, @joshiste. I just stumbled across the article that @izeye linked to as well.

philwebb · 2018-02-21T19:49:48Z

I always preferred the syntax of the zero-array better. I've raised #12160 to switch to it.

joshiste · 2018-02-21T21:45:52Z

My explanation wasn't quite correct...

If the toArray() method instantiates the array itself it can completely omit the initialisation of the array as it knows that it will set the complete content.
If you instantiate the array beforehand, the JVM will zero init the array, because it doesn't know that it will be completly set afterwards... So using a correct sized array results in a initialisation which won't happen when using zero sized arrays

…g/2016/arrays-wisdom-ancients/ See spring-projects/spring-boot#12153

izeye · 2018-02-22T06:02:54Z

I did read the above mentioned article closely and I think I understand the benchmark result better now which looks counter-intuitive at first glance. I'd recommend anyone interested in this topic to read the article.

@joshiste Thanks for enlightening me on this!

Use sized array for List.toArray()

1f610a6

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 21, 2018

rajadilipkolli reviewed Feb 21, 2018

View reviewed changes

philwebb mentioned this pull request Feb 21, 2018

Use zero size array for Collection.toArray calls #12160

Closed

philwebb closed this Feb 21, 2018

philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 21, 2018

izeye added a commit to izeye/sample-jmh-maven that referenced this pull request Feb 22, 2018

Add a benchmark for ArrayList.toArray() from https://shipilev.net/blo…

699803a

…g/2016/arrays-wisdom-ancients/ See spring-projects/spring-boot#12153

izeye deleted the array branch February 22, 2018 06:01

neetkee mentioned this pull request Apr 15, 2019

Use empty array in toArray call. spring-projects/spring-kafka#1069

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use sized array for List.toArray() #12153

Use sized array for List.toArray() #12153

Uh oh!

izeye commented Feb 21, 2018

Uh oh!

rajadilipkolli Feb 21, 2018

Uh oh!

wilkinsona Feb 21, 2018 •

edited

Loading

Uh oh!

rajadilipkolli Feb 21, 2018 •

edited

Loading

Uh oh!

joshiste commented Feb 21, 2018 •

edited

Loading

Uh oh!

izeye commented Feb 21, 2018

Uh oh!

wilkinsona commented Feb 21, 2018 •

edited

Loading

Uh oh!

joshiste commented Feb 21, 2018 •

edited

Loading

Uh oh!

joshiste commented Feb 21, 2018 •

edited

Loading

Uh oh!

izeye commented Feb 21, 2018 •

edited

Loading

Uh oh!

wilkinsona commented Feb 21, 2018

Uh oh!

philwebb commented Feb 21, 2018

Uh oh!

joshiste commented Feb 21, 2018 •

edited

Loading

Uh oh!

izeye commented Feb 22, 2018

Uh oh!

Uh oh!

Use sized array for List.toArray() #12153

Use sized array for List.toArray() #12153

Uh oh!

Conversation

izeye commented Feb 21, 2018

Uh oh!

rajadilipkolli Feb 21, 2018

Choose a reason for hiding this comment

Uh oh!

wilkinsona Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajadilipkolli Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshiste commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

izeye commented Feb 21, 2018

Uh oh!

wilkinsona commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshiste commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshiste commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

izeye commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilkinsona commented Feb 21, 2018

Uh oh!

philwebb commented Feb 21, 2018

Uh oh!

joshiste commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

izeye commented Feb 22, 2018

Uh oh!

Uh oh!

wilkinsona Feb 21, 2018 •

edited

Loading

rajadilipkolli Feb 21, 2018 •

edited

Loading

joshiste commented Feb 21, 2018 •

edited

Loading

wilkinsona commented Feb 21, 2018 •

edited

Loading

joshiste commented Feb 21, 2018 •

edited

Loading

joshiste commented Feb 21, 2018 •

edited

Loading

izeye commented Feb 21, 2018 •

edited

Loading

joshiste commented Feb 21, 2018 •

edited

Loading