commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Heger <oliver.he...@oliver-heger.de>
Subject Re: [lang] Allocate array of the correct size
Date Tue, 05 May 2015 19:32:02 GMT


Am 05.05.2015 um 19:44 schrieb Benedikt Ritter:
> 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
> 

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


Mime
View raw message