git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[JGIT PATCH 09/13] Replace inefficient new String(String) constructor to silence FindBugs

From
Shawn O. Pearce <spearce@spearce.org>
Date
Apr 28, 2009, 21:12 UTC
Message-ID
<1240953146-12878-10-git-send-email-spearce@spearce.org>
In-Reply-To
<1240953146-12878-9-git-send-email-spearce@spearce.org>

FindBugs keeps reporting that our usage of new String(String) is not the most efficient way to construct a string.

http://thread.gmane.org/gmane.comp.version-control.git/113739/focus=113787
Show 11 quoted lines
> 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.

Another way to force this new unique String instance with its own private char[] is to use a StringBuilder and append onto it the ref name. This shouldn't be a warning for FindBugs, but it would accomplish the same goal of producing 1 clean copy, with no extra transient temporary array.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Yann Simon <yann.simon.fr@gmail.com>
CC: Matthias Sohn <matthias.sohn@sap.com>
---
 .../src/org/spearce/jgit/lib/RefDatabase.java      |    3 ++-
 1 files changed, 2 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..bccf2d0 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,8 @@ 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 = new StringBuilder(p.length() - (sp + 1))
+							.append(p, sp + 1, p.length()).toString();
 					last = new Ref(Ref.Storage.PACKED, name, name, id);
 					newPackedRefs.put(last.getName(), last);
 				}
-- 
1.6.3.rc1.205.g37f8
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 10 of 18 in “Misc. bug fixes and cleanups”
  1. 00/13 Misc. bug fixes and cleanupsShawn O. Pearce, Apr 28, 2009
  2. 01/13 Fix performance problem recently introduced to DeltaPackedObjectLoaderShawn O. Pearce, Apr 28, 2009
  3. 02/13 Don't use ByteWindows when checking pack file headers/footersShawn O. Pearce, Apr 28, 2009
  4. 03/13 Rewrite WindowCache to be easier to follow and maintainShawn O. Pearce, Apr 28, 2009
  5. 04/13 Document the IllegalArgumentException thrown by WindowCache.reconfigureShawn O. Pearce, Apr 28, 2009
  6. 05/13 Create the new WindowCache before clearing the old oneShawn O. Pearce, Apr 28, 2009
  7. 06/13 Better handle concurrent reads during a WindowCache reconfigurationShawn O. Pearce, Apr 28, 2009
  8. 07/13 Clear dead OffsetCache cells when clearing a referenceShawn O. Pearce, Apr 28, 2009
  9. 08/13 Work around Sun JVM bug "Cleared SoftReference not added to queue"Shawn O. Pearce, Apr 28, 2009
  10. 09/13 Replace inefficient new String(String) constructor to silence FindBugsShawn O. Pearce, Apr 28, 2009
  11. 10/13 Replace inefficient new Long(long) constructor to silence FindBugsShawn O. Pearce, Apr 28, 2009
  12. 11/13 Change IndexPack to use ObjectIdSubclassMap instead of ObjectIdMapShawn O. Pearce, Apr 28, 2009
  13. 12/13 Use getCachedBytes in IndexPack to avoid an unnecessary copyShawn O. Pearce, Apr 28, 2009
  14. 13/13 Remove ObjectIdMap from the JGit libraryShawn O. Pearce, Apr 28, 2009
  15. Robin RosenbergApr 29, 2009
  16. 12/11 Add a test for LongMap, IndexPack's helper classShawn O. Pearce, Apr 29, 2009
  17. Robin RosenbergApr 29, 2009
  18. Shawn O. PearceApr 29, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.