patch committed at r629320. Thank you.
On 2/20/08, Jimmy,Jing Lv <[EMAIL PROTECTED]> wrote:
> +1 :)
>
> 2008/2/19, Tony Wu <[EMAIL PROTECTED]>:
> > On 2/19/08, Alexei Zakharov <[EMAIL PROTECTED]> wrote:
> > > Right. So my solution is to add a synchronization to all accessors of
> > > this instance variable. Your solution is to create a new object. I
> > > agree that your solution is probably better because it is faster. The
> > > only point I do not completely agree with you is your statement that
> > >
> > > > Actually your proposal doesn't work
> > >
> > > :) Because IMO it works too.
> >
> > Sorry for my abruptness :)
> >
> > >
> > > I'm +1 for your patch to be committed.
> >
> > That's great. Thanks.
> >
> > >
> > > Thanks,
> > > Alexei
> > >
> > > 2008/2/19, Tony Wu <[EMAIL PROTECTED]>:
> > > > On 2/19/08, Alexei Zakharov <[EMAIL PROTECTED]> wrote:
> > > > > Tony,
> > > > >
> > > > > I'm not sure I understand you completely. I didn't find any places in
> > > > > the spec that require creation of a new MessageFormat instance.
> > > > The caching makes it possible to share an instance of MessageFormat
> > > > between several threads, which may cause the synchronization problem
> > > > because the internal status is inconsistence when do formating. The
> > > > instance variable used during formating operation may be modified by
> > > > other thread. To solve this problem we have two choices, one is to
> > > > make the formating operation be synchronized, another is to create new
> > > > instance every time.
> > > >
> > > > > However, at the same by looking at the code I see that such
> > > > > implementation of caching is inefficient by itself because
> > > > > format.toPattern() has the same (or even greater) level of complexity
> > > > > if compared with MessageFormat's constructor. So I tend to agree that
> > > > > creation of a new instance makes sense here.
> > > >
> > > > yes, it's not efficient.
> > > > >
> > > > > Alexei
> > > > >
> > > > > 2008/2/19, Tony Wu <[EMAIL PROTECTED]>:
> > > > > > Hi, Alexei
> > > > > > Actually your proposal doesn't work, the problem is that we can not
> > > > > > caching a format instance here rather than the caching mechanism.
> > > > > > Each
> > > > > > call to this static method must create a new instance of
> > > > > > MessageFormat
> > > > > > (which has been done by ICU already) then do the formating in order
> > > > > > to
> > > > > > sync the access to instance variable otherwise we need synchronize
> > > > > > the
> > > > > > format method itself. Obviously creating a new instance has higher
> > > > > > performance.
> > > > > >
> > > > > > On 2/19/08, Alexei Zakharov <[EMAIL PROTECTED]> wrote:
> > > > > > > Hi Tony,
> > > > > > >
> > > > > > > Don't you think that placing a synchronized block around the
> > > > > > > existing
> > > > > > > code would be a more transparent fix rather than simply
> > > > > > > redirecting
> > > > > > > all job to ICU? I mean something like this:
> > > > > > >
> > > > > > > Index: src/main/java/java/text/MessageFormat.java
> > > > > > > ===================================================================
> > > > > > > --- src/main/java/java/text/MessageFormat.java (revision 628711)
> > > > > > > +++ src/main/java/java/text/MessageFormat.java (working copy)
> > > > > > > @@ -446,6 +446,8 @@
> > > > > > > * when the pattern cannot be parsed
> > > > > > > */
> > > > > > > public static String format(String template, Object...
> > > > > > > objects) {
> > > > > > > + String result;
> > > > > > > +
> > > > > > > if (objects != null) {
> > > > > > > for (int i = 0; i < objects.length; i++) {
> > > > > > > if (objects[i] == null) {
> > > > > > > @@ -453,12 +455,16 @@
> > > > > > > }
> > > > > > > }
> > > > > > > }
> > > > > > > - if (format == null) {
> > > > > > > - format = new
> > > > > > > com.ibm.icu.text.MessageFormat(template);
> > > > > > > - } else if (!template.equals(format.toPattern())){
> > > > > > > - format.applyPattern(template);
> > > > > > > + synchronized (MessageFormat.class) {
> > > > > > > + if (format == null) {
> > > > > > > + format = new
> > > > > > > com.ibm.icu.text.MessageFormat(template);
> > > > > > > + } else if (!template.equals(format.toPattern())){
> > > > > > > + format.applyPattern(template);
> > > > > > > + }
> > > > > > > + result = format.format(objects);
> > > > > > > }
> > > > > > > - return format.format(objects);
> > > > > > > +
> > > > > > > + return result;
> > > > > > > }
> > > > > > >
> > > > > > > /**
> > > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Alexei
> > > > > > >
> > > > > > > 2008/2/19, Tony Wu <[EMAIL PROTECTED]>:
> > > > > > > > This is a major problem which blocks MessageFormat running on
> > > > > > > > multi-thread platform. It fails because the caching in Harmony
> > > > > > > > does
> > > > > > > > not work with ICU4j 3.8.1. I suggest to commit this patch for
> > > > > > > > M5.
> > > > > > > >
> > > > > > > > --
> > > > > > > > Tony Wu
> > > > > > > > China Software Development Lab, IBM
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Tony Wu
> > > > > > China Software Development Lab, IBM
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Tony Wu
> > > > China Software Development Lab, IBM
> > > >
> > >
> >
> >
> > --
> > Tony Wu
> > China Software Development Lab, IBM
> >
>
>
> --
>
> Best Regards!
>
> Jimmy, Jing Lv
> China Software Development Lab, IBM
>
--
Tony Wu
China Software Development Lab, IBM
