All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg.lists@dewire.com>
Cc: git@vger.kernel.org, Yann Simon <yann.simon.fr@gmail.com>,
	Matthias Sohn <matthias.sohn@sap.com>
Subject: Re: [JGIT PATCH 09/13] Replace inefficient new String(String) constructor to silence FindBugs
Date: Wed, 29 Apr 2009 13:22:42 -0700	[thread overview]
Message-ID: <20090429202242.GI23604@spearce.org> (raw)
In-Reply-To: <200904292210.49627.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> tisdag 28 april 2009 23:12:22 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > FindBugs keeps reporting that our usage of new String(String)
> > is not the most efficient way to construct a string.
> 
> I think we should find better ways of silencing FindBugs,, than addiing obscure
> coding patterns that are worse than what FindBugs warns against. 

Heh.  Yea, well...  I also wasn't too happy with FindBugs for
this one.

As far as I can tell there isn't anything in the documentation that
suggests that new String(String) behaves the way I want it to here.

It seems a JRE may be free to reuse the same internal char[] as
the source string, and just produce a new String wrapper.  What I
really want is a deep copy of that char[] to shed what I know is
garbage around the interesting part.

The use of StringBuilder makes this sort of anti-optimization
more difficult, as most JRE implementations would likely
assume they should alloc the internal char[] at the size
given in the constructor, and will deep-copy the chars during
append(String,int,int) because they would expect to see more
characters appended after this append call.

Perhaps the only way to really enforce the behavior I want here is
to convert the String segment to a char[], and then convert that
char[] into a String.  Ick, that's two copies.

Maybe we just stick a comment here.  Two different people have
come up with the same FindBugs issue, trying to get them to share
configuration files sounds hard.
 
> Options are: 
> 	Add a comment 
> 	Customize findbugs rules
> 	Findbugs specific annotations

-- 
Shawn.

      reply	other threads:[~2009-04-29 20:23 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-28 21:12 [JGIT PATCH 00/13] Misc. bug fixes and cleanups Shawn O. Pearce
2009-04-28 21:12 ` [JGIT PATCH 01/13] Fix performance problem recently introduced to DeltaPackedObjectLoader Shawn O. Pearce
2009-04-28 21:12   ` [JGIT PATCH 02/13] Don't use ByteWindows when checking pack file headers/footers Shawn O. Pearce
2009-04-28 21:12     ` [JGIT PATCH 03/13] Rewrite WindowCache to be easier to follow and maintain Shawn O. Pearce
2009-04-28 21:12       ` [JGIT PATCH 04/13] Document the IllegalArgumentException thrown by WindowCache.reconfigure Shawn O. Pearce
2009-04-28 21:12         ` [JGIT PATCH 05/13] Create the new WindowCache before clearing the old one Shawn O. Pearce
2009-04-28 21:12           ` [JGIT PATCH 06/13] Better handle concurrent reads during a WindowCache reconfiguration Shawn O. Pearce
2009-04-28 21:12             ` [JGIT PATCH 07/13] Clear dead OffsetCache cells when clearing a reference Shawn O. Pearce
2009-04-28 21:12               ` [JGIT PATCH 08/13] Work around Sun JVM bug "Cleared SoftReference not added to queue" Shawn O. Pearce
2009-04-28 21:12                 ` [JGIT PATCH 09/13] Replace inefficient new String(String) constructor to silence FindBugs Shawn O. Pearce
2009-04-28 21:12                   ` [JGIT PATCH 10/13] Replace inefficient new Long(long) " Shawn O. Pearce
2009-04-28 21:12                     ` [JGIT PATCH 11/13] Change IndexPack to use ObjectIdSubclassMap instead of ObjectIdMap Shawn O. Pearce
2009-04-28 21:12                       ` [JGIT PATCH 12/13] Use getCachedBytes in IndexPack to avoid an unnecessary copy Shawn O. Pearce
2009-04-28 21:12                         ` [JGIT PATCH 13/13] Remove ObjectIdMap from the JGit library Shawn O. Pearce
2009-04-29 19:45                     ` [JGIT PATCH 10/13] Replace inefficient new Long(long) constructor to silence FindBugs Robin Rosenberg
2009-04-29 20:42                       ` [PATCH 12/11] Add a test for LongMap, IndexPack's helper class Shawn O. Pearce
2009-04-29 20:10                   ` [JGIT PATCH 09/13] Replace inefficient new String(String) constructor to silence FindBugs Robin Rosenberg
2009-04-29 20:22                     ` Shawn O. Pearce [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090429202242.GI23604@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=matthias.sohn@sap.com \
    --cc=robin.rosenberg.lists@dewire.com \
    --cc=yann.simon.fr@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.