yep .. I didn't see that one. That makes sense. Thanks for clarifying. In that case, a line in the class doco for each of those "not thread safe .. do not cache one instance anywhere" wouldn't hurt.
Actually in my case it wasn't even the snippet I posted causing the problem, since that map only contains MY type converters that coincidentally are all thread safe ... After a second look, I found another class that had a field, private IntegerTypeConverter intConverter; ... can't do that... had to delete the field and pull it from the factory inside the method call (reinstantiate it) instead of pulling it once in the constructor. That smells but really it's the JDKs fault. I just dislike it when any classes in a web application aren't thread safe, because you never know how you might want to use something. ________________________________________ From: Ben Gunter [bgun...@cpons.com] Sent: Saturday, November 19, 2011 7:50 AM To: Stripes Users List Subject: Re: [Stripes-users] Subclassing TypeConverterFactory -- newInstance() I did this and committed it years ago. Then Tim saw the change and told me he already had a JIRA issue for it that should have been resolved Won't Fix. He had done some testing and found that it really doesn't help enough to be worth the added complexity. So he reverted it. See for reference: http://www.stripesframework.org/jira/browse/STS-42?page=com.atlassian.jira.plugin.system.issuetabpanels%3Aall-tabpanel#issue-tabs http://stripes.svn.sourceforge.net/viewvc/stripes?view=revision&revision=682 http://stripes.svn.sourceforge.net/viewvc/stripes?view=revision&revision=712 -Ben On Fri, Nov 18, 2011 at 4:12 PM, Newman, John W <newma...@d3onc.com<mailto:newma...@d3onc.com>> wrote: All, I just wanted to sound off about this as I know I borrowed this snippet of code from someone on this mailing list a long time ago, and I think there may be a few others out there still using it. Subclassing the type converter factory to cache the instances instead of reinstating every time like so: public class TypeConverterFactory extends DefaultTypeConverterFactory { private static Map<Class<? extends TypeConverter<?>>, TypeConverter<?>> converterInstanceMap = new ConcurrentHashMap<Class<? extends TypeConverter<?>>, TypeConverter<?>>(); @SuppressWarnings("unchecked") // super method uses raw type .. should be <?> @Override public TypeConverter getInstance(Class<? extends TypeConverter> clazz, Locale locale) throws Exception { TypeConverter<?> typeConverter = converterInstanceMap.get(clazz); return typeConverter == null ? super.getInstance(clazz, locale) : typeConverter; } } .. is a bad idea, as it saves instances of the DateTypeConverter and NumberTypeConverter which are NOT thread safe. I’ve had to remove this to get my load tests to pass. At first I thought it was a bug that those are not thread safe, then I noticed this instance cache. I’d like to propose that the default converter works this way to avoid this inefficient instantiation on every request, and that the DateTypeConverter and NumberTypeConverter either synchronize on the SimpleDateFormat and NumberFormat instances or pool them. The sync approach would be a pretty easy patch, I’d be happy to submit it, pooling would be better but would be more complex and probably add a dependency. Any thoughts on that? Thanks, John ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-novd2d _______________________________________________ Stripes-users mailing list Stripes-users@lists.sourceforge.net<mailto:Stripes-users@lists.sourceforge.net> https://lists.sourceforge.net/lists/listinfo/stripes-users ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-novd2d _______________________________________________ Stripes-users mailing list Stripes-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/stripes-users
