You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Johan Compagner <jc...@gmail.com> on 2008/03/23 16:43:31 UTC
VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
I was looking to fix this issue:
https://issues.apache.org/jira/browse/WICKET-1421
and then i came across AbstractDecimalConverter which still caches Format
objects
Those are not thread safe so we should really remove those.
The problem is then we also have to remove setNumberFormat(LocalemFormat)
method
and that is an API break. (That method could be used if you as developer
know for SURE that the converter is used in 1 thread at a time)
Much better would be to override getNumberFormat() and return what every you
want for that locale, so that method can have bad consequents to use anyway.
so again:
[] remove the map and the method for 1.3
[] remove the map and the method for 1.4
johan
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
Posted by Eelco Hillenius <ee...@gmail.com>.
[ ] remove the map and the method for 1.3
[ x ] remove the map and the method for 1.4
I don't think users benefit much from being able to set the formatter
directly anyway.
Eelco
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats
fields that holds NumberFormats, But those are not thread save...
Posted by Martin Grigorov <mc...@e-card.bg>.
[] remove the map and the method for 1.3
[X] remove the map and the method for 1.4
Non-binding
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
Posted by Johan Compagner <jc...@gmail.com>.
hmm
i am really not a fan for also using thread locals for that
Then we have to use a threadlocal map that hold per locale a number format..
I dont know if clone is really faster then just new (ok the parsing doesn't
have to happen then)
Then i could keep the map and always return a cloned version of that..
But will that gain much?
johan
On Sun, Mar 23, 2008 at 6:48 PM, James Carman <ja...@carmanconsulting.com>
wrote:
> NumberFormat objects are cloneable. So, maybe you could use the
> format passed in to setNumberFormat() as a "prototype" to create one
> for the current thread/invocation (I don't know if you use thread
> locals for this or not)?
>
> On Sun, Mar 23, 2008 at 11:43 AM, Johan Compagner <jc...@gmail.com>
> wrote:
> > I was looking to fix this issue:
> > https://issues.apache.org/jira/browse/WICKET-1421
> >
> > and then i came across AbstractDecimalConverter which still caches
> Format
> > objects
> > Those are not thread safe so we should really remove those.
> > The problem is then we also have to remove
> setNumberFormat(LocalemFormat)
> > method
> > and that is an API break. (That method could be used if you as
> developer
> > know for SURE that the converter is used in 1 thread at a time)
> > Much better would be to override getNumberFormat() and return what
> every you
> > want for that locale, so that method can have bad consequents to use
> anyway.
> > so again:
> >
> > [] remove the map and the method for 1.3
> > [] remove the map and the method for 1.4
> >
> > johan
> >
>
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Mar 23, 2008 at 5:47 PM, Johan Compagner <jc...@gmail.com> wrote:
> hmm for 1.3 i will test it that i will always just give a cloned version
> Then we can keep the map and the method just fine.
>
> That fix is fine by me then
>
That sounds like it will work. I don't know what would be faster, but
perhaps you could benchmark it. You could try a few approaches...
1. Cloning the "prototype" DecimalFormat object.
2. Just saving the pattern and DecimalFormatStrings from the passed
in DecimalFormat and use those to construct a new one each time.
3. Synchronization (one one DecimalFormat instance). They're not
used for that long.
4. ThreadLocal (does Wicket have any sort of facility for clearing
thread locals at the end of a request cycle like Tapestry does/did?)
5. Some sort of "pool"
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
Posted by Johan Compagner <jc...@gmail.com>.
hmm for 1.3 i will test it that i will always just give a cloned version
Then we can keep the map and the method just fine.
That fix is fine by me then
johan
On Sun, Mar 23, 2008 at 6:48 PM, James Carman <ja...@carmanconsulting.com>
wrote:
> NumberFormat objects are cloneable. So, maybe you could use the
> format passed in to setNumberFormat() as a "prototype" to create one
> for the current thread/invocation (I don't know if you use thread
> locals for this or not)?
>
> On Sun, Mar 23, 2008 at 11:43 AM, Johan Compagner <jc...@gmail.com>
> wrote:
> > I was looking to fix this issue:
> > https://issues.apache.org/jira/browse/WICKET-1421
> >
> > and then i came across AbstractDecimalConverter which still caches
> Format
> > objects
> > Those are not thread safe so we should really remove those.
> > The problem is then we also have to remove
> setNumberFormat(LocalemFormat)
> > method
> > and that is an API break. (That method could be used if you as
> developer
> > know for SURE that the converter is used in 1 thread at a time)
> > Much better would be to override getNumberFormat() and return what
> every you
> > want for that locale, so that method can have bad consequents to use
> anyway.
> > so again:
> >
> > [] remove the map and the method for 1.3
> > [] remove the map and the method for 1.4
> >
> > johan
> >
>
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
Posted by James Carman <ja...@carmanconsulting.com>.
NumberFormat objects are cloneable. So, maybe you could use the
format passed in to setNumberFormat() as a "prototype" to create one
for the current thread/invocation (I don't know if you use thread
locals for this or not)?
On Sun, Mar 23, 2008 at 11:43 AM, Johan Compagner <jc...@gmail.com> wrote:
> I was looking to fix this issue:
> https://issues.apache.org/jira/browse/WICKET-1421
>
> and then i came across AbstractDecimalConverter which still caches Format
> objects
> Those are not thread safe so we should really remove those.
> The problem is then we also have to remove setNumberFormat(LocalemFormat)
> method
> and that is an API break. (That method could be used if you as developer
> know for SURE that the converter is used in 1 thread at a time)
> Much better would be to override getNumberFormat() and return what every you
> want for that locale, so that method can have bad consequents to use anyway.
> so again:
>
> [] remove the map and the method for 1.3
> [] remove the map and the method for 1.4
>
> johan
>
Re: VOTE issue 1444, AbstractDecimalConverter has a numberFormats fields that holds NumberFormats, But those are not thread save...
Posted by Nick Heudecker <nh...@gmail.com>.
>
> [] remove the map and the method for 1.3
> [X] remove the map and the method for 1.4
>
>
Non-binding.