Skip to main content
Code Review

Return to Question

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Because of 1., a thread local cache should cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question this question. For the same reason no custom class may be put into the ThreadLocal.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

Notice removed Draw attention by Community Bot
Bounty Ended with toto2's answer chosen by Community Bot
added 397 characters in body
Source Link
maaartinus
  • 13.6k
  • 1
  • 35
  • 73

Concerning the optimization of this simple function

String formatDate(String format, Locale locale, Date date) {...}

formatting a Date using a SimpleDateFormat created with the arguments format and locale. I'm curious what you thing about the code below. I know it's terrible, but let's assume two things

  1. The code must be as fast as possible.
  2. There's no suitable object to hold the cache.
  3. It must not cause any memory leaks.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 2., the cache must be a static field.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

This all is pretty restrictive. So I wrote the following ugliness

private String toString(Date obj, String format, Locale locale) {
 return getSimpleDateFormat(format, locale).format(obj);
}
private SimpleDateFormat getSimpleDateFormat(String format, Locale locale) {
 Object[] list = SDF.get();
 // An identity check is faster and correct as missing an entry is allowed.
 // It's surely good enough as passing equals but not same parameters hardly ever happens.
 if (list!=null && list[0] == format && list[1] == locale) {
 return (SimpleDateFormat) list[2];
 }
 final SimpleDateFormat result = new SimpleDateFormat(format, locale);
 list = new Object[] {format, locale, result};
 SDF.set(list);
 return result;
}
/** Contains triples (String format, Locale locale, SimpleDateFormat sdf) */
private static final ThreadLocal<Object[]> SDF = new ThreadLocal<Object[]>();

I don't care about naming and such, as this was just a quick implementation of an idea. I'm curious if there's a substantially better solution satisfying all the above conditions.

If you say, it's premature optimization, I'll fully agree, but let's assume for a while it must be done. If you say, that's hardly faster than no caching, then show me your figures.


Obviously, I'm caching just one value per thread. It should be enough in many cases like producing a CSV with one one or more dates per row as I haven't yet see them to be formatted differently.

However, time is "date" too (at least in Java), and a CSV line containing both date and time in separate columns is rather common and this would render my poor man's cache fully ineffective.

Concerning the optimization of this simple function

String formatDate(String format, Locale locale, Date date) {...}

formatting a Date using a SimpleDateFormat created with the arguments format and locale. I'm curious what you thing about the code below. I know it's terrible, but let's assume two things

  1. The code must be as fast as possible.
  2. There's no suitable object to hold the cache.
  3. It must not cause any memory leaks.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 2., the cache must be a static field.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

This all is pretty restrictive. So I wrote the following ugliness

private String toString(Date obj, String format, Locale locale) {
 return getSimpleDateFormat(format, locale).format(obj);
}
private SimpleDateFormat getSimpleDateFormat(String format, Locale locale) {
 Object[] list = SDF.get();
 // An identity check is faster and correct as missing an entry is allowed.
 // It's surely good enough as passing equals but not same parameters hardly ever happens.
 if (list!=null && list[0] == format && list[1] == locale) {
 return (SimpleDateFormat) list[2];
 }
 final SimpleDateFormat result = new SimpleDateFormat(format, locale);
 list = new Object[] {format, locale, result};
 SDF.set(list);
 return result;
}
/** Contains triples (String format, Locale locale, SimpleDateFormat sdf) */
private static final ThreadLocal<Object[]> SDF = new ThreadLocal<Object[]>();

I don't care about naming and such, as this was just a quick implementation of an idea. I'm curious if there's a substantially better solution satisfying all the above conditions.

If you say, it's premature optimization, I'll fully agree, but let's assume for a while it must be done. If you say, that's hardly faster than no caching, then show me your figures.

Concerning the optimization of this simple function

String formatDate(String format, Locale locale, Date date) {...}

formatting a Date using a SimpleDateFormat created with the arguments format and locale. I'm curious what you thing about the code below. I know it's terrible, but let's assume two things

  1. The code must be as fast as possible.
  2. There's no suitable object to hold the cache.
  3. It must not cause any memory leaks.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 2., the cache must be a static field.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

This all is pretty restrictive. So I wrote the following ugliness

private String toString(Date obj, String format, Locale locale) {
 return getSimpleDateFormat(format, locale).format(obj);
}
private SimpleDateFormat getSimpleDateFormat(String format, Locale locale) {
 Object[] list = SDF.get();
 // An identity check is faster and correct as missing an entry is allowed.
 // It's surely good enough as passing equals but not same parameters hardly ever happens.
 if (list!=null && list[0] == format && list[1] == locale) {
 return (SimpleDateFormat) list[2];
 }
 final SimpleDateFormat result = new SimpleDateFormat(format, locale);
 list = new Object[] {format, locale, result};
 SDF.set(list);
 return result;
}
/** Contains triples (String format, Locale locale, SimpleDateFormat sdf) */
private static final ThreadLocal<Object[]> SDF = new ThreadLocal<Object[]>();

I don't care about naming and such, as this was just a quick implementation of an idea. I'm curious if there's a substantially better solution satisfying all the above conditions.

If you say, it's premature optimization, I'll fully agree, but let's assume for a while it must be done. If you say, that's hardly faster than no caching, then show me your figures.


Obviously, I'm caching just one value per thread. It should be enough in many cases like producing a CSV with one one or more dates per row as I haven't yet see them to be formatted differently.

However, time is "date" too (at least in Java), and a CSV line containing both date and time in separate columns is rather common and this would render my poor man's cache fully ineffective.

Tweeted twitter.com/#!/StackCodeReview/status/513391014803800064
Notice added Draw attention by maaartinus
Bounty Started worth 50 reputation by maaartinus
Remove something superfluous.
Source Link
maaartinus
  • 13.6k
  • 1
  • 35
  • 73

Concerning the optimization of this simple function

String formatDate(String format, Locale locale, Date date) {...}

formatting a Date using a SimpleDateFormat created with the arguments format and locale. I'm curious what you thing about the code below. I know it's terrible, but let's assume two things

  1. The code must be as fast as possible.
  2. There's no suitable object to hold the cache.
  3. It must not cause any memory leaks.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 2., the cache must be a static field.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

This all is pretty restrictive. So I wrote the following ugliness

private String toString(Date obj, String format, Locale locale) {
 if (format.isEmpty()) return obj.toString();
 checkArgument(format.startsWith("sdf:"));
 return getSimpleDateFormat(format.substring(4), locale).format(obj);
}
private SimpleDateFormat getSimpleDateFormat(String format, Locale locale) {
 Object[] list = SDF.get();
 // An identity check is faster and correct as missing an entry is allowed.
 // It's surely good enough as passing equals but not same parameters hardly ever happens.
 if (list!=null && list[0] == format && list[1] == locale) {
 return (SimpleDateFormat) list[2];
 }
 final SimpleDateFormat result = new SimpleDateFormat(format, locale);
 list = new Object[] {format, locale, result};
 SDF.set(list);
 return result;
}
/** Contains triples (String format, Locale locale, SimpleDateFormat sdf) */
private static final ThreadLocal<Object[]> SDF = new ThreadLocal<Object[]>();

I don't care about naming and such, as this was just a quick implementation of an idea. I'm curious if there's a substantially better solution satisfying all the above conditions.

If you say, it's premature optimization, I'll fully agree, but let's assume for a while it must be done. If you say, that's hardly faster than no caching, then show me your figures.

Concerning the optimization of this simple function

String formatDate(String format, Locale locale, Date date) {...}

formatting a Date using a SimpleDateFormat created with the arguments format and locale. I'm curious what you thing about the code below. I know it's terrible, but let's assume two things

  1. The code must be as fast as possible.
  2. There's no suitable object to hold the cache.
  3. It must not cause any memory leaks.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 2., the cache must be a static field.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

This all is pretty restrictive. So I wrote the following ugliness

private String toString(Date obj, String format, Locale locale) {
 if (format.isEmpty()) return obj.toString();
 checkArgument(format.startsWith("sdf:"));
 return getSimpleDateFormat(format.substring(4), locale).format(obj);
}
private SimpleDateFormat getSimpleDateFormat(String format, Locale locale) {
 Object[] list = SDF.get();
 // An identity check is faster and correct as missing an entry is allowed.
 // It's surely good enough as passing equals but not same parameters hardly ever happens.
 if (list!=null && list[0] == format && list[1] == locale) {
 return (SimpleDateFormat) list[2];
 }
 final SimpleDateFormat result = new SimpleDateFormat(format, locale);
 list = new Object[] {format, locale, result};
 SDF.set(list);
 return result;
}
/** Contains triples (String format, Locale locale, SimpleDateFormat sdf) */
private static final ThreadLocal<Object[]> SDF = new ThreadLocal<Object[]>();

I don't care about naming and such, as this was just a quick implementation of an idea. I'm curious if there's a substantially better solution satisfying all the above conditions.

If you say, it's premature optimization, I'll fully agree, but let's assume for a while it must be done. If you say, that's hardly faster than no caching, then show me your figures.

Concerning the optimization of this simple function

String formatDate(String format, Locale locale, Date date) {...}

formatting a Date using a SimpleDateFormat created with the arguments format and locale. I'm curious what you thing about the code below. I know it's terrible, but let's assume two things

  1. The code must be as fast as possible.
  2. There's no suitable object to hold the cache.
  3. It must not cause any memory leaks.

Because of 1., a thread local cache should be used. However, the passed format and arguments may differ from those used in the cache and this must be checked. Funnily, although SimpleDateFormat is mutable (which is the cause of all problems), there seem to be no way how to change its format or locale. Actually, the parameters hardly ever change, but there's no guarantee.

Because of 2., the cache must be a static field.

Because of 3., no ThreadLocal.initialValue can be used as it'd lead to memory leaks just like in this question. For the same reason no custom class may be put into the ThreadLocal.

This all is pretty restrictive. So I wrote the following ugliness

private String toString(Date obj, String format, Locale locale) {
 return getSimpleDateFormat(format, locale).format(obj);
}
private SimpleDateFormat getSimpleDateFormat(String format, Locale locale) {
 Object[] list = SDF.get();
 // An identity check is faster and correct as missing an entry is allowed.
 // It's surely good enough as passing equals but not same parameters hardly ever happens.
 if (list!=null && list[0] == format && list[1] == locale) {
 return (SimpleDateFormat) list[2];
 }
 final SimpleDateFormat result = new SimpleDateFormat(format, locale);
 list = new Object[] {format, locale, result};
 SDF.set(list);
 return result;
}
/** Contains triples (String format, Locale locale, SimpleDateFormat sdf) */
private static final ThreadLocal<Object[]> SDF = new ThreadLocal<Object[]>();

I don't care about naming and such, as this was just a quick implementation of an idea. I'm curious if there's a substantially better solution satisfying all the above conditions.

If you say, it's premature optimization, I'll fully agree, but let's assume for a while it must be done. If you say, that's hardly faster than no caching, then show me your figures.

edited tags
Link
200_success
  • 145.5k
  • 22
  • 190
  • 478
Loading
Switch to an array as there's no point in using an list.
Source Link
maaartinus
  • 13.6k
  • 1
  • 35
  • 73
Loading
Source Link
maaartinus
  • 13.6k
  • 1
  • 35
  • 73
Loading
lang-java

AltStyle によって変換されたページ (->オリジナル) /