commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [lang] Allocate array of the correct size
Date Tue, 05 May 2015 17:44:29 GMT
2015-05-05 19:13 GMT+02:00 Jörg Schaible <joerg.schaible@gmx.de>:

> Benedikt Ritter wrote:
>
> > 2015-05-05 17:51 GMT+02:00 Jörg Schaible <joerg.schaible@swisspost.com>:
> >
> >> Hi Benedikt,
> >>
> >> Benedikt Ritter wrote:
> >>
> >> > 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <britter@apache.org>:
> >> >
> >> >> Hello Jörg,
> >> >>
> >> >> 2015-05-05 8:30 GMT+02:00 Jörg Schaible
> >> >> <joerg.schaible@swisspost.com>:
> >> >>
> >> >>> Hi Benedikt,
> >> >>>
> >> >>> britter@apache.org wrote:
> >> >>>
> >> >>> > Repository: commons-lang
> >> >>> > Updated Branches:
> >> >>> >   refs/heads/master 8548b12d8 -> 60b32953a
> >> >>> >
> >> >>> >
> >> >>> > Allocate array of the correct size
> >> >>> >
> >> >>> >
> >> >>> > Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
> >> >>> > Commit:
> >> >>> >
> http://git-wip-us.apache.org/repos/asf/commons-lang/commit/60b32953
> >> >>> Tree:
> >> >>> > http://git-wip-us.apache.org/repos/asf/commons-lang/tree/60b32953
> >> >>> > Diff:
> >> >>> > http://git-wip-us.apache.org/repos/asf/commons-lang/diff/60b32953
> >> >>> >
> >> >>> > Branch: refs/heads/master
> >> >>> > Commit: 60b32953a968e5623f82a6b27d6c679bc17c48e5
> >> >>> > Parents: 8548b12
> >> >>> > Author: Benedikt Ritter <britter@apache.org>
> >> >>> > Authored: Mon May 4 21:26:07 2015 +0200
> >> >>> > Committer: Benedikt Ritter
> >> >>> > <britter@apache.org> Committed: Mon May 4
> >> >>> > 21:26:07 2015 +0200
> >> >>> >
> >> >>> >
> >> ----------------------------------------------------------------------
> >> >>> >  .../apache/commons/lang3/builder/ReflectionToStringBuilder.java
> >> >>> >  | 2
> >> >>> +-
> >> >>> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>> >
> >> ----------------------------------------------------------------------
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>>
> >>
> http://git-wip-us.apache.org/repos/asf/commons-lang/blob/60b32953/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java
> >> >>> >
> >> ----------------------------------------------------------------------
> >> >>> > diff --git
> >> >>> >
> >> >>>
> >> >>>
> >>
> >>
>
> a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java
> >> >>> >
> >> >>>
> >> >>>
> >>
> >>
>
> b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java
> >> >>> > index 5904469..7a78170 100644 ---
> >> >>> >
> >> >>>
> >> >>>
> >>
> >>
>
> a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java
> >> >>> > +++
> >> >>> >
> >> >>>
> >> >>>
> >>
> >>
>
> b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java
> >> >>> > @@ -333,7 +333,7 @@ public class ReflectionToStringBuilder
extends
> >> >>> > ToStringBuilder {
> >> >>> >                  list.add(e.toString());
> >> >>> >              }
> >> >>> >          }
> >> >>> > -        return list.toArray(ArrayUtils.EMPTY_STRING_ARRAY);
> >> >>> > +        return list.toArray(new String[list.size()]);
> >> >>> >      }
> >> >>>
> >> >>> What's the benefit of this? Where's the difference by letting
> >> >>> List.toArray()
> >> >>> allocate the appropriate array compared to do it on your own?
> >> >>> ArrayUtils.EMPTY_STRING is a constant after all, so there's no
> >> >>> additional allocation.
> >> >>>
> >> >>
> >> >> I changed this because my IDE complained about that line of code:
> >> >>
> >> >> "Call to 'toArray' with zero-length array argument
> >> >> 'ArrayUtils.EMPTY_STRING_ARRAY'
> >> >>
> >> >> Reports any call to 'toArray' on an object or type or subtype of
> >> >> java.util.Collection with a zero-length argument. When passing an
> >> >> array of too small size, the toArray() method has to construct a new
> >> >> array of the correct size using reflection. This has significantly
> >> >> worse performance than passing in an array of at least the size of
> the
> >> >> collection itself."
> >> >>
> >> >> To be honest, I did not do any performance benchmarks to make sure
> >> >> this is really true.
> >> >>
> >> >
> >> > In any case, the commit message should have been more explanatory.
> >> > Sorry about that.
> >>
> >> Well, that warning is somewhat stupid, if you're using a constant for
> the
> >> zero length array. The "worse performance" only occurs if you provide a
> >> new array instance that is too small.
> >>
> >
> > ... which will always be the case unless the list is empty, or am I
> > missing something here?
>
> Where's the difference in creating a new array of proper size yourself or
> let the method do it? It's even worse now, because now you create a new
> instance *even* if the list is empty.
>

The difference is, that toArray(T[]) will have to create a new instance
using reflection every time the
ReflectionToStringBuilder.toNoNullStringArray(Object[]) method is invoked
with an non empty array (see ArrayList.toArray(T[]), line 389). The IDE
report complains that this will be significantly slower then creating a new
array of the correct type and size using an array constructor. As I said, I
haven't done any benchmarks. But it seemed logical to me.

br,
Benedikt


>
> Cheers,
> Jörg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message