harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nathan Beyer" <nbe...@kc.rr.com>
Subject RE: [classlib] Approval for String patch for HARMONY-719
Date Mon, 03 Jul 2006 18:22:06 GMT
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.

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.

-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


Mime
View raw message