Is it worth commenting that Strings.common* expose potential memory leak from JDK String? - guava-discuss

The old Google Groups will be going away soon, but your browser is incompatible with the new version.
Message from discussion Is it worth commenting that Strings.common* expose potential memory leak from JDK String?
Received-SPF: pass (google.com: domain of cgdec...@google.com designates 209.85.216.53 as permitted sender) client-ip=209.85.216.53;
MIME-Version: 1.0
In-Reply-To: <CAFGi0fHVQUDqrtbjXabm0tz9LfnYfWwp+ZDeRXrV8aHU=v3...@mail.gmail.com>
References: <310a0f15-2c35-4061-b7c2-35d5153e420f@googlegroups.com>
 <CA+tXMXNHgerrE3cOcfrfOup2Rj+i1xQi6Bq63zzt8j0oga_...@mail.gmail.com>
 <CAFGi0fExCEti9f=1qpC_x6D__2KQg7NsDMbaNd50Y6u4m-U...@mail.gmail.com>
 <CA+tXMXMVBMkkwG7ZVBcC7vjkqBgMQEM=eXMFjPPiczUiRMK...@mail.gmail.com>
 <dc163809-7a1a-4a9d-a1d0-5701d866ea75@googlegroups.com> <CAFGi0fHVQUDqrtbjXabm0tz9LfnYfWwp+ZDeRXrV8aHU=v3...@mail.gmail.com>
From: Colin Decker <cgdec...@google.com>
Date: Fri, 29 Jun 2012 10:38:28 -0400
Message-ID: <CAHJY=p7-wsqDYx0i3eOia5Nao_fO0HKdbaVtJHS=8Ufe8eQ...@mail.gmail.com>
Subject: Re: [guava] Is it worth commenting that Strings.common* expose
 potential memory leak from JDK String?
To: Osvaldo Doederlein <opin...@google.com>
Cc: Ian Robertson <ian.b.robert...@gmail.com>, guava-discuss@googlegroups.com, 
	Emily Soldal <em...@soldal.org>
Content-Type: multipart/alternative; boundary=20cf303344bff4fee004c39d6768

--20cf303344bff4fee004c39d6768
Content-Type: text/plain; charset=ISO-8859-1

If I recall correctly, String's hashCode function has been documented all
along, which is precisely why they haven't been able to change it to
something better.


On Fri, Jun 29, 2012 at 6:14 AM, Osvaldo Doederlein <opin...@google.com>wrote:

> The only thing that could go wrong with the new implementation is worse
> performance for code that makes lots of substring operations where the
> source string is not discarded, so the heap has to keep two copies of the
> common characters. More exotic scenarios (involving JNI or privileged
> reflection) are illegal and would cause failure in the current impl too.
>
> Making the copy/view decision dynamically is not viable because the main
> objective of this change is to get rid of the offset and count fields, plus
> the extra code to handle them; the new String class becomes just a wrapper
> over a single field with the char[].  (Well, that and the cached hash too,
> they tried to get rid of that but the experiment was not successful.)  The
> savings from the two removed fields are significant, Strings are typically
> the most common objects in Java heaps and most Strings are small -- see the
> classic paper http://www.cs.ucsb.edu/~urs/oocsb/papers/TRCS98-33.pdf,
> maybe Urs wants to update us on this ;)
>
> Frankly this change should have come years before, if not for
> conservatism. Too bad that Sun/Oracle didn't also bit the bullet to fix
> String's very bad hashCode function, just the opposite they recently
> documented it because the traditional function had become a de-facto
> standard and some weird stuff would break if that function changed.
>
> A+
> Osvaldo
>
>
>
> On Fri, Jun 29, 2012 at 2:19 AM, Ian Robertson <ian.b.robert...@gmail.com>wrote:
>
>> One of the few changes Azul made to the core JDK classes in their open
>> sourced Managed Runtime Initiative was a similar change to
>> java.lang.String. I was never able to find any documentation as to why,
>> however.
>>
>>
>> On Tuesday, June 26, 2012 7:22:57 AM UTC-6, Louis Wasserman wrote:
>>>
>>> ...Huh.
>>>
>>> That seems like a change that could go wrong in lots of different ways.
>>>  Mind, I can think of slightly more reasonable alternatives -- for example,
>>> always doing the copy if the substring is small, but just doing the view if
>>> the substring is comparatively large.
>>>
>>
>  --
> guava-discuss@googlegroups.com
> Project site: http://guava-libraries.googlecode.com
> This group: http://groups.google.com/group/guava-discuss
>
> This list is for general discussion.
> To report an issue: http://code.google.com/p/guava-libraries/issues/entry
> To get help: http://stackoverflow.com/questions/ask (use the tag "guava")
>



-- 
Colin Decker | Software Engineer | cgdec...@google.com | Java Core
Libraries Team

--20cf303344bff4fee004c39d6768
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

If I recall correctly, String&#39;s hashCode function has been documented a=
ll along, which is precisely why they haven&#39;t been able to change it to=
 something better.<div class=3D"gmail_extra"><br><br><div class=3D"gmail_qu=
ote">

On Fri, Jun 29, 2012 at 6:14 AM, Osvaldo Doederlein <span dir=3D"ltr">&lt;<=
a href=3D"mailto:opin...@google.com" target=3D"_blank">opin...@google.com</=
a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0=
 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

The only thing that could go wrong with the new implementation is worse per=
formance for code that makes lots of substring operations where the source =
string is not discarded, so the heap has to keep two copies of the common c=
haracters. More exotic scenarios (involving JNI or privileged reflection) a=
re illegal and would cause failure in the current impl too.<div>


<br></div><div>Making the copy/view decision dynamically is not viable beca=
use the main objective of this change is to get rid of the offset and count=
 fields, plus the extra code to handle them; the new String class becomes j=
ust a wrapper over a single field with the char[]. =A0(Well, that and the c=
ached hash too, they tried to get rid of that but the experiment was not su=
ccessful.) =A0The savings from the two removed fields are significant, Stri=
ngs are typically the most common objects in Java heaps and most Strings ar=
e small -- see the classic paper=A0<a href=3D"http://www.cs.ucsb.edu/~urs/o=
ocsb/papers/TRCS98-33.pdf" target=3D"_blank">http://www.cs.ucsb.edu/~urs/oo=
csb/papers/TRCS98-33.pdf</a>, maybe Urs wants to update us on this ;)</div>


<div><br></div><div>Frankly this change should have come years before, if n=
ot for conservatism. Too bad that Sun/Oracle didn&#39;t also bit the bullet=
 to fix String&#39;s very bad hashCode function, just the opposite they rec=
ently documented it because the traditional function had become a de-facto =
standard and some weird stuff would break if that function changed.</div>


<div><br></div><div>A+</div><div><span class=3D"HOEnZb"><font color=3D"#888=
888">Osvaldo</font></span><div><div class=3D"h5"><br><div class=3D"gmail_ex=
tra"><br><br><div class=3D"gmail_quote">On Fri, Jun 29, 2012 at 2:19 AM, Ia=
n Robertson <span dir=3D"ltr">&lt;<a href=3D"mailto:ian.b.robert...@gmail.c=
om" target=3D"_blank">ian.b.robert...@gmail.com</a>&gt;</span> wrote:<br>


<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">One of the few changes Azul made to the core=
 JDK classes in their open sourced Managed Runtime Initiative was=A0a simil=
ar change to java.lang.String. I was never able to find any documentation a=
s to why, however.<div>


<div><br><br>On Tuesday, June 26, 2012 7:22:57 AM UTC-6, Louis Wasserman wr=
ote:<blockquote class=3D"gmail_quote" style=3D"margin:0;margin-left:0.8ex;b=
order-left:1px #ccc solid;padding-left:1ex">...Huh.<div><br></div>
<div>That seems like a change that could go wrong in lots of different ways=
. =A0Mind, I can think of slightly more reasonable alternatives -- for exam=
ple, always doing the copy if the substring is small, but just doing the vi=
ew if the substring is comparatively large.</div>



</blockquote></div></div></blockquote></div><br></div></div></div></div><di=
v class=3D"HOEnZb"><div class=3D"h5">

<p></p>

-- <br>
<a href=3D"mailto:guava-discuss@googlegroups.com" target=3D"_blank">guava-d=
iscuss@googlegroups.com</a><br>
Project site: <a href=3D"http://guava-libraries.googlecode.com" target=3D"_=
blank">http://guava-libraries.googlecode.com</a><br>
This group: <a href=3D"http://groups.google.com/group/guava-discuss" target=
=3D"_blank">http://groups.google.com/group/guava-discuss</a><br>
=A0<br>
This list is for general discussion.<br>
To report an issue: <a href=3D"http://code.google.com/p/guava-libraries/iss=
ues/entry" target=3D"_blank">http://code.google.com/p/guava-libraries/issue=
s/entry</a><br>
To get help: <a href=3D"http://stackoverflow.com/questions/ask" target=3D"_=
blank">http://stackoverflow.com/questions/ask</a> (use the tag &quot;guava&=
quot;)<br>
</div></div></blockquote></div><br><br clear=3D"all"><div><br></div>-- <br>=
<span style=3D"color:rgb(85,85,85);font-family:sans-serif;font-size:small;l=
ine-height:1.5em;border-width:2px 0px 0px;border-style:solid;border-color:r=
gb(213,15,37);padding-top:2px;margin-top:2px">Colin Decker=A0|</span><span =
style=3D"color:rgb(85,85,85);font-family:sans-serif;font-size:small;line-he=
ight:1.5em;border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,=
105,232);padding-top:2px;margin-top:2px">=A0Software Engineer=A0|</span><sp=
an style=3D"color:rgb(85,85,85);font-family:sans-serif;font-size:small;line=
-height:1.5em;border-width:2px 0px 0px;border-style:solid;border-color:rgb(=
0,153,57);padding-top:2px;margin-top:2px">=A0<a href=3D"mailto:cgdecker@goo=
gle.com" target=3D"_blank">cgdec...@google.com</a>=A0|</span><span style=3D=
"color:rgb(85,85,85);font-family:sans-serif;font-size:small;line-height:1.5=
em;border-width:2px 0px 0px;border-style:solid;border-color:rgb(238,178,17)=
;padding-top:2px;margin-top:2px">=A0Java Core Libraries Team</span><br>

<br>
</div>

--20cf303344bff4fee004c39d6768--
Create a group - Google Groups - Google Home - Terms of Service - Privacy Policy
Account Options
