You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Jörg Schaible <jo...@swisspost.com> on 2015/05/05 08:30:42 UTC

Re: [lang] Allocate array of the correct size

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 <br...@apache.org>
> Authored: Mon May 4 21:26:07 2015 +0200
> Committer: Benedikt Ritter
> <br...@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.

- Jörg



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


Re: [lang] Allocate array of the correct size

Posted by Peter Ansell <an...@gmail.com>.
On 6 May 2015 at 01:51, Jörg Schaible <jo...@swisspost.com> wrote:
> Hi Benedikt,
>
> Benedikt Ritter wrote:
>
>> 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
>>
>>> Hello Jörg,
>>>
>>> 2015-05-05 8:30 GMT+02:00 Jörg Schaible <jo...@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 <br...@apache.org>
>>>> > Authored: Mon May 4 21:26:07 2015 +0200
>>>> > Committer: Benedikt Ritter
>>>> > <br...@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.

The worse performance would be in reflection overhead as reflection
may always be used to derive the class for a new array if you are
passing in an empty array. There is no need for any microbenchmarks on
different JVMs if reflection is used at all in the zero length array
method on any JVM. The reflection method will necessarily have more
CPU instructions than a native array allocation of the correct size
which is done in compiled code and JIT'ed without any reflection
calls.

Cheers,

Peter

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


Re: [lang] Allocate array of the correct size

Posted by Jörg Schaible <jo...@gmx.de>.
Benedikt Ritter wrote:

> 2015-05-05 19:13 GMT+02:00 Jörg Schaible <jo...@gmx.de>:
> 
>> Benedikt Ritter wrote:
>>
>> > 2015-05-05 17:51 GMT+02:00 Jörg Schaible
>> > <jo...@swisspost.com>:
>> >
>> >> Hi Benedikt,
>> >>
>> >> Benedikt Ritter wrote:
>> >>
>> >> > 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
>> >> >
>> >> >> Hello Jörg,
>> >> >>
>> >> >> 2015-05-05 8:30 GMT+02:00 Jörg Schaible
>> >> >> <jo...@swisspost.com>:
>> >> >>
>> >> >>> Hi Benedikt,
>> >> >>>

[snip]

>> >> >>> 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.

OK, agreed. Only a micro benchmark on the different Java runtimes will tell. 
And it might be different for Collection implementations than ArrayList.
Nevertheless, I was just curious about the reasoning.

Cheers,
Jörg


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


Re: [lang] Allocate array of the correct size

Posted by Oliver Heger <ol...@oliver-heger.de>.

Am 05.05.2015 um 19:44 schrieb Benedikt Ritter:
> 2015-05-05 19:13 GMT+02:00 Jörg Schaible <jo...@gmx.de>:
> 
>> Benedikt Ritter wrote:
>>
>>> 2015-05-05 17:51 GMT+02:00 Jörg Schaible <jo...@swisspost.com>:
>>>
>>>> Hi Benedikt,
>>>>
>>>> Benedikt Ritter wrote:
>>>>
>>>>> 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
>>>>>
>>>>>> Hello Jörg,
>>>>>>
>>>>>> 2015-05-05 8:30 GMT+02:00 Jörg Schaible
>>>>>> <jo...@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 <br...@apache.org>
>>>>>>>> Authored: Mon May 4 21:26:07 2015 +0200
>>>>>>>> Committer: Benedikt Ritter
>>>>>>>> <br...@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
> 

ISTR that also Bloch in "Effective Java" recommended the approach of
passing in a zero-length array (which can be a constant as it is
immutable) and let the method create a properly sized array itself.

Oliver

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

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


Re: [lang] Allocate array of the correct size

Posted by Benedikt Ritter <br...@apache.org>.
2015-05-05 19:13 GMT+02:00 Jörg Schaible <jo...@gmx.de>:

> Benedikt Ritter wrote:
>
> > 2015-05-05 17:51 GMT+02:00 Jörg Schaible <jo...@swisspost.com>:
> >
> >> Hi Benedikt,
> >>
> >> Benedikt Ritter wrote:
> >>
> >> > 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
> >> >
> >> >> Hello Jörg,
> >> >>
> >> >> 2015-05-05 8:30 GMT+02:00 Jörg Schaible
> >> >> <jo...@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 <br...@apache.org>
> >> >>> > Authored: Mon May 4 21:26:07 2015 +0200
> >> >>> > Committer: Benedikt Ritter
> >> >>> > <br...@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

Re: [lang] Allocate array of the correct size

Posted by Jörg Schaible <jo...@gmx.de>.
Benedikt Ritter wrote:

> 2015-05-05 17:51 GMT+02:00 Jörg Schaible <jo...@swisspost.com>:
> 
>> Hi Benedikt,
>>
>> Benedikt Ritter wrote:
>>
>> > 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
>> >
>> >> Hello Jörg,
>> >>
>> >> 2015-05-05 8:30 GMT+02:00 Jörg Schaible
>> >> <jo...@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 <br...@apache.org>
>> >>> > Authored: Mon May 4 21:26:07 2015 +0200
>> >>> > Committer: Benedikt Ritter
>> >>> > <br...@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.

Cheers,
Jörg


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


Re: [lang] Allocate array of the correct size

Posted by Benedikt Ritter <br...@apache.org>.
2015-05-05 17:51 GMT+02:00 Jörg Schaible <jo...@swisspost.com>:

> Hi Benedikt,
>
> Benedikt Ritter wrote:
>
> > 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
> >
> >> Hello Jörg,
> >>
> >> 2015-05-05 8:30 GMT+02:00 Jörg Schaible <jo...@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 <br...@apache.org>
> >>> > Authored: Mon May 4 21:26:07 2015 +0200
> >>> > Committer: Benedikt Ritter
> >>> > <br...@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?

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

Re: [lang] Allocate array of the correct size

Posted by Jörg Schaible <jo...@swisspost.com>.
Hi Benedikt,

Benedikt Ritter wrote:

> 2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:
> 
>> Hello Jörg,
>>
>> 2015-05-05 8:30 GMT+02:00 Jörg Schaible <jo...@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 <br...@apache.org>
>>> > Authored: Mon May 4 21:26:07 2015 +0200
>>> > Committer: Benedikt Ritter
>>> > <br...@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.

Cheers,
Jörg


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


Re: [lang] Allocate array of the correct size

Posted by Benedikt Ritter <br...@apache.org>.
2015-05-05 14:52 GMT+02:00 Benedikt Ritter <br...@apache.org>:

> Hello Jörg,
>
> 2015-05-05 8:30 GMT+02:00 Jörg Schaible <jo...@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 <br...@apache.org>
>> > Authored: Mon May 4 21:26:07 2015 +0200
>> > Committer: Benedikt Ritter
>> > <br...@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.


>
> Benedikt
>
>
>>
>> - 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
>



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

Re: [lang] Allocate array of the correct size

Posted by Benedikt Ritter <br...@apache.org>.
Hello Jörg,

2015-05-05 8:30 GMT+02:00 Jörg Schaible <jo...@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 <br...@apache.org>
> > Authored: Mon May 4 21:26:07 2015 +0200
> > Committer: Benedikt Ritter
> > <br...@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.

Benedikt


>
> - 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