49

In an effort to reduce mutability, should we rather use

public void setValues(String[] newVals) {

     this.vals = ( newVals == null ? null : newVals.clone() );
}

or

public void setValues(String[] newVals) {

     this.vals = ( newVals == null ? null : Arrays.copyOf(newVals, newVals.length) );
}
Bruno Grieder
  • 28,128
  • 8
  • 69
  • 101

4 Answers4

44

Update using jmh

Using jmh, I get similar results, except that clone seems to be marginally better.

Original post

I ran a quick test for performance: clone, System.arrayCopy and Arrays.copyOf have very similar performance (jdk 1.7.06, server vm).

For details (in ms), after JIT:

clone: 68
arrayCopy: 68
Arrays.copyOf: 68

Test code:

public static void main(String[] args) throws InterruptedException,
        IOException {
    int sum = 0;
    int[] warmup = new int[1];
    warmup[0] = 1;
    for (int i = 0; i < 15000; i++) { // triggers JIT
        sum += copyClone(warmup);
        sum += copyArrayCopy(warmup);
        sum += copyCopyOf(warmup);
    }

    int count = 10_000_000;
    int[] array = new int[count];
    for (int i = 0; i < count; i++) {
        array[i] = i;
    }

    // additional warmup for main
    for (int i = 0; i < 10; i++) {
        sum += copyArrayCopy(array);
    }
    System.gc();
    // copyClone
    long start = System.nanoTime();
    for (int i = 0; i < 10; i++) {
        sum += copyClone(array);
    }
    long end = System.nanoTime();
    System.out.println("clone: " + (end - start) / 1000000);
    System.gc();
    // copyArrayCopy
    start = System.nanoTime();
    for (int i = 0; i < 10; i++) {
        sum += copyArrayCopy(array);
    }
    end = System.nanoTime();
    System.out.println("arrayCopy: " + (end - start) / 1000000);
    System.gc();
    // copyCopyOf
    start = System.nanoTime();
    for (int i = 0; i < 10; i++) {
        sum += copyCopyOf(array);
    }
    end = System.nanoTime();
    System.out.println("Arrays.copyOf: " + (end - start) / 1000000);
    // sum
    System.out.println(sum);
}

private static int copyClone(int[] array) {
    int[] copy = array.clone();
    return copy[copy.length - 1];
}

private static int copyArrayCopy(int[] array) {
    int[] copy = new int[array.length];
    System.arraycopy(array, 0, copy, 0, array.length);
    return copy[copy.length - 1];
}

private static int copyCopyOf(int[] array) {
    int[] copy = Arrays.copyOf(array, array.length);
    return copy[copy.length - 1];
}
Community
  • 1
  • 1
assylias
  • 321,522
  • 82
  • 660
  • 783
  • 18
    ... and the conclusion is that `clone()` is the best option, being a nilary method (no parameters to screw up) which also is as typesafe as it can get (thanks to return type covariance on `clone()` for array types). – gustafc Aug 28 '12 at 13:42
  • There is a junit plugin for doing benchmarks like this from Carrot Search Labs - [JUnitBenchmarks](http://labs.carrotsearch.com/junit-benchmarks.html). It makes creating micro benchmarks like this very easy. – Christian Trimble Dec 20 '12 at 15:23
  • @C.Trimble I have used [caliper](http://code.google.com/p/caliper/) in the past and it works well. – assylias Dec 20 '12 at 15:27
  • @assylias I just read the caliper docs and it looks very interesting. – Christian Trimble Dec 20 '12 at 23:50
  • 4
    Arrays.copyOf uses System.arrayCopy internally so no need to comparison between them. – Bharat DEVre Jul 04 '16 at 08:49
7

Please also consider the security of using "clone()". A class of well-known attacks use classes that override objects' "clone()" methods with malicious code. For example, CVE-2012-0507 (the "Flashback" attack on Mac OS) was addressed by basically replacing a ".clone()" call with a ".copyOf" call.

Additional discussion on the obsolete-ness of "clone()" can be found on StackOverflow here: object cloning with out implementing cloneable interface

Community
  • 1
  • 1
Dalek Control
  • 587
  • 5
  • 7
  • Late but out of curiosity, is is possible to synthetically extend an Array type. Certainly you can't do it using regular Java syntax. – Valentin Ruano Jan 01 '20 at 17:17
  • Looking at the case that you link too, seems strange that it would ever work.... perhaps aggressive compiler optimization may result in not checking that the value passed to ```E[]``` is in fact an array since the inlined code only executes the ```clone``` which is supported by all objects. – Valentin Ruano Jan 01 '20 at 17:28
  • I see, this is in fact only possible by low-level invalid manipulation of object pointers called "***type-confusion***". – Valentin Ruano Jan 01 '20 at 17:33
4

I written a simple program to check the difference.

public static void main(String[] args) throws IOException, InterruptedException,
        PrinterException
{
  //Verify array remains immutable.

  String[] str =  {"a","b","c"};
  String[] strings  = str.clone();
  //change returned array
  strings[2]= "d";
  System.out.println(Arrays.toString(str));
  System.out.println(Arrays.toString(strings));

  String[] stringsCopy = Arrays.copyOf(str, str.length);
  stringsCopy[2]= "d";
  System.out.println(Arrays.toString(str));
  System.out.println(Arrays.toString(stringsCopy));

  //peformance
  long before = System.currentTimeMillis();
  for(int i=0;i<Integer.MAX_VALUE;i++)
  {
      str.clone();
  }
  System.out.println("Time Required for Clone: "+ (System.currentTimeMillis()-before));

  //peformance
  long beforeCopy = System.currentTimeMillis();
  for(int i=0;i<Integer.MAX_VALUE;i++)
  {
      Arrays.copyOf(str, str.length);
  }
  System.out.println("Time Required for Copy of: "+ (System.currentTimeMillis()-beforeCopy));

}

And it outputs

[a, b, c]
[a, b, d]
[a, b, c]
[a, b, d]
Time Required for Clone: 26288
Time Required for Copy of: 25413

So if you see in both case String[] is immutable and performance is almost same thought Arrays.copyOf() is slightly faster on my machine.

Update

I changed program to create large array[100 strings] rather than small array.

  String[] str =  new String[100];

  for(int i= 0; i<str.length;i++)
  {
      str[i]= Integer.toString(i);
  }

And moved copy of method before clone method. With below results.

 Time Required for Copy of: 415095
 Time Required for Clone: 428501

Which are again more of same. Please do not ask me to run the test again as it takes a while :(

Update 2

For String array 1000000 and for number of iterations 10000

Time Required for Copy of: 32825
Time Required for Clone: 30138

copy of takes more time than clone

Amit Deshpande
  • 19,001
  • 4
  • 46
  • 72
  • @assylias for Integer.MAX_VALUE ie `0x7fffffff` – Amit Deshpande Aug 28 '12 at 10:45
  • I mean how many times have you run the whole benchmark? Results could vary from one run to another - Have you also made sure GC is not interering with your results? – assylias Aug 28 '12 at 10:46
  • @assylias 2 times and yes they vary but if we take an average then it is almost same for both cases. – Amit Deshpande Aug 28 '12 at 10:48
  • This copies a very small array many times. It is far better to test copying of large arrays in order to stress the copying itself and not any logic surrounding it. – Marko Topolnik Aug 28 '12 at 10:57
  • @AmitD Do you get the same result if you switch the loops (Arrays.copyOf first, then clone)? – assylias Aug 28 '12 at 11:03
  • @Marko Topolnik,@assylias I have updated program to address both suggestions but because of length of array is more it is taking a while. I will update the result once program finishes. – Amit Deshpande Aug 28 '12 at 11:19
  • Reduce the number of iterations. You don't need billions of iterations. – Marko Topolnik Aug 28 '12 at 11:24
  • A hundred elements is not exactly large, it's still smallish. Ten thousand would qualify for a medium-sized array, and a million and above would be considered large. But nevermind, your results are in line with all the other results around here. – Marko Topolnik Aug 28 '12 at 11:29
  • @MarkoTopolnik Update 2 for array size Million. – Amit Deshpande Aug 28 '12 at 11:38
  • Without careful use of `System.gc` your results can be significantly skewed. You should rerun the whole program for like ten times and see that the results are fully stable, and test that they are stable under the reordering of benchmarks (test clone first, or test copyOf first). – Marko Topolnik Aug 28 '12 at 11:40
  • @AmitD From your latest updates I suppose we can conclude that the 2 methods are equivalent. +1 for the hard work ;-) – assylias Aug 28 '12 at 11:50
  • Thank you very much for your answer and your detailed work on the subject. – Bruno Grieder Aug 28 '12 at 13:30
1

In terms of mutability, they will provide exactly the same - shallow copy of data.

jdevelop
  • 12,176
  • 10
  • 56
  • 112
  • 3
    Shallow copy of immutable objects is a perfectly valid approach. – Marko Topolnik Aug 28 '12 at 10:29
  • 1
    @BGR copyOf uses System.arraycopy, I guess that clone uses it as well. So they should have equal performance (still, copyOf has some additional checks for types, so it might be slightly slower) – jdevelop Aug 28 '12 at 10:33
  • `copyOf` needs type checking only if you specify the resulting array type. Similar for `System.arraycopy`. – Marko Topolnik Aug 28 '12 at 10:55