Re: [commons-lang] branch master updated: Use Collection#toArray(new T[0]) instead of a presized array as it is faster on modern JVMs.

Sun, 29 Dec 2019 04:38:30 -0800

One possible reason to use T[0] is because that's how the new (since Java 11) toArray method does it. From java.util.Collection, OpenJDK version 13: 

    default <T> T[] toArray(IntFunction<T[]> generator) {
        return toArray(generator.apply(0));
    }
This method has, as of yet, not been overridden in any of the implementations in java.base. 

On 29/12/2019 01:19, sebb wrote:

On Fri, 27 Dec 2019 at 17:43, Matt Sicker <boa...@gmail.com> wrote:


It's somewhat logical, though, that T[0] is faster than T[size]. In
both cases, the given array is checked to see if it's large enough to
fit the collection contents. If it isn't, then a new array is
allocated. By deferring the new array allocation operation to the
toArray() method call, you avoid two array length checks.



I don't think the size check is relevant.

AFAICT the reason modern JVMs are slower when passed T[size] is that the
code no longer copies the collection directly into the provided array, but
uses a temporary work array instead.
The comments indicate that this is to protect against malicious
multi-threaded code getting access to the collection.
AFAIK the original JVM versions of the code did not have this protection.

TL;DR: this is probably a spurious micro-optimization.




Agreed.



On Fri, 27 Dec 2019 at 08:48, Gary Gregory <garydgreg...@gmail.com> wrote:


I wonder where this should be documented. We could add a // comment to

each

call site, that seems the simplest. Adding a package or component level
documentation in the style guideline for example could easily be

overlooked

by someone in a debugger for example. So each call site would be best IMO
something like
// Using an empty array can be faster
I don't really want a link to an external random blog in the source.

Thoughts?

Then we can consider doing it throughout Commons.

Gary

On Fri, Dec 27, 2019 at 7:23 AM Pascal Schumacher <

pascalschumac...@gmx.net>

wrote:


Feel free to revert the changes.

I would revert my changes, but Garry did some follow-up changes so

these

would to be reverted first.

Am 27.12.2019 um 12:33 schrieb sebb:

As that article says in conclusion, T[0] seems currently faster, but

that

may not always be the case with future VMs.

Also it says that IntelliJ IDEA and PMD recommend using T[size]

rather

than

T[0].
If we release code with T[0], I suspect we will get complaints that

the

code is not efficient according to x, y and z.

I'm not convinced that the change is worth it, but if it is agreed

the

code

must be thoroughly documented to try and forestall complaints.
According to the article, using a constant empty array is faster, but

only

marginally. If there is a constant available it would probably make

sense

to use it.
This should also be documented, as it affects the return value for

the

empty case.

S.
On Fri, 27 Dec 2019 at 09:28, Pascal Schumacher <

pascalschumac...@gmx.net>

wrote:


see https://shipilev.net/blog/2016/arrays-wisdom-ancients/

Am 27.12.2019 um 01:24 schrieb sebb:

Also, where is it documented that modern JVMs are faster?
To which JVMs does this apply?

S.

On Thu, 26 Dec 2019 at 22:08, Gary Gregory <garydgreg...@gmail.com



wrote:

Please do not cause garbage to apparently be generated all over

the

place

by creating new empty arrays all the time. Use the constants

Commons

Lang

constants already defines; see ArrayUtils.

Gary

On Thu, Dec 26, 2019 at 4:48 PM <pascalschumac...@apache.org>

wrote:



This is an automated email from the ASF dual-hosted git

repository.


pascalschumacher pushed a commit to branch master
in repository

https://gitbox.apache.org/repos/asf/commons-lang.git



The following commit(s) were added to refs/heads/master by this

push:

        new 84668a2  Use Collection#toArray(new T[0]) instead of a

presized

array as it is faster on modern JVMs.
84668a2 is described below

commit 84668a2d980316a580030fd64764cb072b520b09
Author: pascalschumacher <pascalschumac...@gmx.net>
AuthorDate: Thu Dec 26 22:48:12 2019 +0100

       Use Collection#toArray(new T[0]) instead of a presized

array as

it is

faster on modern JVMs.
---
    src/main/java/org/apache/commons/lang3/CharSet.java

   |

2

+-

    src/main/java/org/apache/commons/lang3/StringUtils.java

   |

10

+++++-----
    .../org/apache/commons/lang3/exception/ExceptionUtils.java

  |

6

+++---



src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java |

4

++--

    .../java/org/apache/commons/lang3/reflect/MethodUtils.java

  |

2

+-



src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java  |

2

+-



src/main/java/org/apache/commons/lang3/text/StrTokenizer.java  |

4

++--

    .../org/apache/commons/lang3/time/DurationFormatUtils.java

  |

2

+-

    .../java/org/apache/commons/lang3/time/FastDatePrinter.java

   |

2

+-



.../commons/lang3/concurrent/EventCountCircuitBreakerTest.java |

2

+-

    10 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/main/java/org/apache/commons/lang3/CharSet.java
b/src/main/java/org/apache/commons/lang3/CharSet.java
index 3fdfd07..7955115 100644
--- a/src/main/java/org/apache/commons/lang3/CharSet.java
+++ b/src/main/java/org/apache/commons/lang3/CharSet.java
@@ -225,7 +225,7 @@ public class CharSet implements Serializable

{

    // NOTE: This is no longer public as CharRange is no longer a

public

class.
    //       It may be replaced when CharSet moves to Range.
        /*public*/ CharRange[] getCharRanges() {
-        return set.toArray(new CharRange[set.size()]);
+        return set.toArray(new CharRange[0]);
        }








//-----------------------------------------------------------------------

diff --git

a/src/main/java/org/apache/commons/lang3/StringUtils.java

b/src/main/java/org/apache/commons/lang3/StringUtils.java
index abde7ec..d629806 100644
--- a/src/main/java/org/apache/commons/lang3/StringUtils.java
+++ b/src/main/java/org/apache/commons/lang3/StringUtils.java
@@ -7507,7 +7507,7 @@ public class StringUtils {
                currentType = type;
            }
            list.add(new String(c, tokenStart, c.length -

tokenStart));

-        return list.toArray(new String[list.size()]);
+        return list.toArray(new String[0]);
        }

        /**
@@ -7735,7 +7735,7 @@ public class StringUtils {
                }
            }

-        return substrings.toArray(new

String[substrings.size()]);

+        return substrings.toArray(new String[0]);
        }

        //


-----------------------------------------------------------------------

@@ -7923,7 +7923,7 @@ public class StringUtils {
            if (match || preserveAllTokens && lastMatch) {
                list.add(str.substring(start, i));
            }
-        return list.toArray(new String[list.size()]);
+        return list.toArray(new String[0]);
        }

        /**
@@ -8022,7 +8022,7 @@ public class StringUtils {
            if (match || preserveAllTokens && lastMatch) {
                list.add(str.substring(start, i));
            }
-        return list.toArray(new String[list.size()]);
+        return list.toArray(new String[0]);
        }

        /**
@@ -8835,7 +8835,7 @@ public class StringUtils {
            if (list.isEmpty()) {
                return null;
            }
-        return list.toArray(new String[list.size()]);
+        return list.toArray(new String[0]);
        }

        /**
diff --git


a/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java



b/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java

index 5969614..dd154e3 100644
---



a/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java

+++



b/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java

@@ -274,7 +274,7 @@ public class ExceptionUtils {
                }
                frames.addAll(trace);
            }
-        return frames.toArray(new String[frames.size()]);
+        return frames.toArray(new String[0]);
        }

        /**
@@ -325,7 +325,7 @@ public class ExceptionUtils {
            while (frames.hasMoreTokens()) {
                list.add(frames.nextToken());
            }
-        return list.toArray(new String[list.size()]);
+        return list.toArray(new String[0]);
        }

        /**
@@ -438,7 +438,7 @@ public class ExceptionUtils {
         */
        public static Throwable[] getThrowables(final Throwable

throwable) {

            final List<Throwable> list =

getThrowableList(throwable);

-        return list.toArray(new Throwable[list.size()]);
+        return list.toArray(new Throwable[0]);
        }

        /**
diff --git
a/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java
b/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java
index d72dc53..553be4e 100644
---

a/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java

+++

b/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java

@@ -199,7 +199,7 @@ public class FieldUtils {
         */
        public static Field[] getAllFields(final Class<?> cls) {
            final List<Field> allFieldsList =

getAllFieldsList(cls);

-        return allFieldsList.toArray(new

Field[allFieldsList.size()]);

+        return allFieldsList.toArray(new Field[0]);
        }

        /**
@@ -237,7 +237,7 @@ public class FieldUtils {
         */
        public static Field[] getFieldsWithAnnotation(final

Class<?>

cls,

final Class<? extends Annotation> annotationCls) {
            final List<Field> annotatedFieldsList =
getFieldsListWithAnnotation(cls, annotationCls);
-        return annotatedFieldsList.toArray(new
Field[annotatedFieldsList.size()]);
+        return annotatedFieldsList.toArray(new Field[0]);
        }

        /**
diff --git
a/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
b/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
index bbd5019..491470d 100644
---

a/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java

+++

b/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java

@@ -878,7 +878,7 @@ public class MethodUtils {
                                                        final

boolean

searchSupers, final boolean ignoreAccess) {
            final List<Method> annotatedMethodsList =
getMethodsListWithAnnotation(cls, annotationCls, searchSupers,
                    ignoreAccess);
-        return annotatedMethodsList.toArray(new
Method[annotatedMethodsList.size()]);
+        return annotatedMethodsList.toArray(new Method[0]);
        }

        /**
diff --git

a/src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java

b/src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java
index 2a6ccd0..f9df8c6 100644
---

a/src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java

+++

b/src/main/java/org/apache/commons/lang3/reflect/TypeUtils.java

@@ -1149,7 +1149,7 @@ public class TypeUtils {
                }
            }

-        return types.toArray(new Type[types.size()]);
+        return types.toArray(new Type[0]);
        }

        /**
diff --git

a/src/main/java/org/apache/commons/lang3/text/StrTokenizer.java

b/src/main/java/org/apache/commons/lang3/text/StrTokenizer.java
index c9ab666..97fae7d 100644
---

a/src/main/java/org/apache/commons/lang3/text/StrTokenizer.java

+++

b/src/main/java/org/apache/commons/lang3/text/StrTokenizer.java

@@ -604,10 +604,10 @@ public class StrTokenizer implements
ListIterator<String>, Cloneable {
                if (chars == null) {
                    // still call tokenize as subclass may do some

work

                    final List<String> split = tokenize(null, 0,

0);

-                tokens = split.toArray(new

String[split.size()]);

+                tokens = split.toArray(new String[0]);
                } else {
                    final List<String> split = tokenize(chars, 0,
chars.length);
-                tokens = split.toArray(new

String[split.size()]);

+                tokens = split.toArray(new String[0]);
                }
            }
        }
diff --git


a/src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java



b/src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java

index 83af2e0..4a75237 100644
---



a/src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java

+++



b/src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java

@@ -564,7 +564,7 @@ public class DurationFormatUtils {
            if (inLiteral) { // i.e. we have not found the end of

the

literal

                throw new IllegalArgumentException("Unmatched

quote in

format: " + format);
            }
-        return list.toArray(new Token[list.size()]);
+        return list.toArray(new Token[0]);
        }








//-----------------------------------------------------------------------

diff --git


a/src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java



b/src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java

index a5edb28..888d3fb 100644
---

a/src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java

+++

b/src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java

@@ -160,7 +160,7 @@ public class FastDatePrinter implements

DatePrinter,

Serializable {
         */
        private void init() {
            final List<Rule> rulesList = parsePattern();
-        mRules = rulesList.toArray(new Rule[rulesList.size()]);
+        mRules = rulesList.toArray(new Rule[0]);

            int len = 0;
            for (int i=mRules.length; --i >= 0; ) {
diff --git






a/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java





b/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java

index b3fb5cf..2819006 100644
---






a/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java

+++






b/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java

@@ -407,7 +407,7 @@ public class EventCountCircuitBreakerTest {
             */
            public void verify(final Boolean... values) {
                assertArrayEquals(values,
-                    changedValues.toArray(new
Boolean[changedValues.size()]));
+                    changedValues.toArray(new Boolean[0]));
            }
        }
    }






---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org






--
Matt Sicker <boa...@gmail.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to