On Dec 21, 2004, at 9:54 AM, Ceki G�lc� wrote:
CachedDateFormat would not be able to detect the milliseconds field on RelativeTimeDateFormat unless the starting time was an integral second and would not be able to detect millisecond fields if non-arabic digits were set. In either of these cases, you would have an extra call per format evaluation. I believe the original patch avoided caching RelativeTimeDateFormat.
Making DatePatternConverter aware of CachedDateFormat would avoid caching RelativeTimeDateFormat.
I believe the original integration with pattern layout did attempt to cache RelativeTimeDateFormat. I was just saying that it would be technically possible to hand code that combination and if you did, it would just perform a little slower.
The worse-case scenario is if you could construct a date-time format where the location of the millisecond field changed, but the total length of the field did not. I don't think that you could create one with SimpleDateFormat, however you could obviously write a custom DateFormat that did.
It could only happen if at some point in time, some field or field we shorted and one or more fields were shortened by the same amount. If a simple algorithm could be devised to detect such cases beforehand, then CachedDateFormat is a winner.
I had mentioned "SS0" was a potentially malicious time format. In the code where PatternLayout creates the SimpleDateFormat, the format string could be sanity checked. If it appeared troublesome, then the SimpleDateFormat would not be wrapped with CachedDateFormat. If you wanted to be very careful, you'd get skip caching anything containing "G", "MMM", "E", "z" and things with only one or two "S".
Another approach would be to staleness date the millisecond field so that you don't trust it if it is, say, over a minute old.
There is an observable difference when running the performance tests to a null appender with CachedDateFormat. However, it may not be significant in more realistic deployments. It is a significant improvement over the flawed (and currently unused) caching code in the original DateFormats. However, the original motivation for the caching may no longer be relevant and so a new CachedDateFormat may not have a performance benefit that justifies the added complexity.
Mostly agreed. However, I still wonder whether with a little extra work CachedDateFormat could not be polished to become a pearl.
The second pass used localized values of both 0 and 9 to identify the millisecond field. If the default locale changed, CachedDateFormat would not switch locales until the next integral second. There may be other issues that come to pass with any locale rework, so maybe the best approach is to leave CachedDateFormat out for now. It will be available in Bugzilla in case someone ever wants to add it later.
I'll give it a shot if you don't mind.
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
