harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexei Zakharov" <alexei.zakha...@gmail.com>
Subject Re: [Approval] Commit Harmony-5460
Date Tue, 19 Feb 2008 12:12:53 GMT
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.
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.

Alexei

2008/2/19, Tony Wu <wuyuehao@gmail.com>:
> 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 <alexei.zakharov@gmail.com> 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 <wuyuehao@gmail.com>:
> > > 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
>

Mime
View raw message