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

From: Shawn O. Pearce <spearce@spearce.org>
Date: Mar 19, 2009, 16:01 UTC
Message-ID: <20090319160102.GQ23521@spearce.org>
In-Reply-To: <49C20D4E.5020203@gmail.com>

Yann Simon <yann.simon.fr@gmail.com> wrote:

Show 22 quoted lines

> 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@gmail.com>
> ---
>  .../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.

-- 
Shawn.