Re: [PATCH JGIT] Method invokes inefficient new String(String) constructor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2009/3/19 Shawn O. Pearce <spearce@xxxxxxxxxxx>:
> Yann Simon <yann.simon.fr@xxxxxxxxx> wrote:
>> From FindBugs:
>> Using the java.lang.String(String) constructor wastes memory because
>> the object so constructed will be functionally indistinguishable from
>> the String passed as a parameter. Just use the argument String directly.
>>
>> Signed-off-by: Yann Simon <yann.simon.fr@xxxxxxxxx>
>> ---
>>  .../src/org/spearce/jgit/lib/RefDatabase.java      |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
>> index 87f26bf..49da538 100644
>> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
>> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
>> @@ -447,7 +447,7 @@ private synchronized void refreshPackedRefs() {
>>
>>                                       final int sp = p.indexOf(' ');
>>                                       final ObjectId id = ObjectId.fromString(p.substring(0, sp));
>> -                                     final String name = new String(p.substring(sp + 1));
>> +                                     final String name = p.substring(sp + 1);
>>                                       last = new Ref(Ref.Storage.PACKED, name, name, id);
>>                                       newPackedRefs.put(last.getName(), last);
>
> I had a specific reason for forcing a new String object here.
>
> The line in question, p, is from the packed-refs file and
> contains the entire SHA-1 in hex form at the beginning of it.
> We've converted that into binary as an ObjectId, it uses 1/4 the
> space of the string portion.
>
> The Ref object, its ObjectId, and its name string, are going to be
> cached in a Map, probably long-term.  We're better off shedding the
> 80 bytes of memory used to hold the hex SHA-1 then risk substring()
> deciding its "faster" to reuse the char[] then to make a copy of it.

However, using the trick newString = new String(aString.substring(),
i) does not work on all JVM.
With an IBM JVM, the newString will still contain the original array of chars.

Another solution that work on all JVM could be:
newString = new String(aString.substring(i).toCharArray())
Or
newString = new String(aString.toCharArray(), i, aString.length() - i)

I like the latter one.

Yann
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]