harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Geir Magnusson Jr <g...@pobox.com>
Subject Re: [classlib] Approval for String patch for HARMONY-719
Date Mon, 03 Jul 2006 18:57:11 GMT


Nathan Beyer wrote:
> The concern I would have about this is the scenario where it actually is a
> StringBuffer, especially an empty StringBuffer. The point of the instanceof
> check and delegation to the contentEquals(StringBuffer) is to maintain
> synchronization guarantees of StringBuffer. If we call the 'length()' before
> the instanceof check, those calls will take place outside of a single
> synchronized block.

But since StringBuffer is supposed to be threadsafe, do I have to worry
about synchronizing on the object to get a correct value of length() in
the event that cs is a StringBuffer?

> 
> I wrote the original implementation of this method, so the validity and
> correctness of this is definitely up for discussion. I based my decision for
> the "single synchronization" on the implementation of the
> String(StringBuffer) and contentEquals(StringBuffer), which provide
> additional synchronization above that which is inherent to StringBuffer.
> 
> Personally, I've never assumed that the synchronization of StringBuffer
> would every be guaranteed beyond method calls to the class itself, but I can
> understand the rationale for doing this, as you'd want the outcome of the
> methods to be deterministic regardless of concurrency scenarios.

I'm not sure where the lack of determinism comes from for this case -
testing if the CharSequence has a zero length - but I'm interested in
understanding why.

geir

> 
> -Nathan
> 
>> -----Original Message-----
>> From: Geir Magnusson Jr [mailto:geir@pobox.com]
>>
>> Interesting - couldn't you promote this fix as a minor performance
>> improvement as well to knock out would would be a pointless instanceof
>> in the case of ""?
>>
>> public boolean contentEquals(CharSequence cs) {
>>
>>   if (cs == null) {
>> 	throw new NPE;
>>   }
>>
>>   if (cs.length() != count) {
>>         return false;
>>   }
>>
>>   if (cs.length() == 0 && count == 0) {
>>         return true;  // since both are empty strings
>>   }
>>
>>   if (cs instanceof StringBuffer) {
>>         return contentEquals(...);
>>   }
>>   else {
>>     return regionMatches(...);
>>   }
>> }
>>
>>
>> Nathan Beyer wrote:
>>> http://issues.apache.org/jira/browse/HARMONY-719
>>>
>>>
>>>
>>> This issue identifies a valid bug in java.lang.String and good patch to
>> fix
>>> it. Does anyone have any objections to applying this patch or any
>> comments?
>>>
>>>
>>> -Nathan
>>>
>>>
>> ---------------------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> 
> 
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> 
> 
> 

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message