RE: [JGIT PATCH v3] Replace inefficient new String(String) constructor to silence FindBugs
- From
Sohn, Matthias <matthias.sohn@sap.com>
- Date
- May 4, 2009, 21:23 UTC
- Message-ID
- <366BBB1215D0AB4B8A153AF047A2878003073DC3@dewdfe18.wdf.sap.corp>
- In-Reply-To
- <1241193272-20247-1-git-send-email-spearce@spearce.org>
Shawn O. Pearce [mailto:spearce@spearce.org] wrote :
Show 29 quoted lines
> 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 > > 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> > --- > > A less ugly version ?
I agree, this looks better than the previous proposal but still a simple String copy constructor looks even simpler.
I tried the alternative approach Robin proposed using FindBugs filter mechanisms to suppress the undesired warning. I will post that in my next mail.
-- Matthias