Re: TimeZone

[Bryce McKinlay <mckinlay at redhat dot com>]

Hi Mark,

This is great, thanks for working on it. I have a few comments after 
looking through the patch:

- We are making very posix-y (or even glibc?) assumptions in the generic 
java.util.TimeZone code, specifically the /etc/localtime and 
/etc/timezone reading code. Perhaps this should be factored out into a 
posix-specific file? Windows, for example, will probably have a saner 
way to get the timezone and will want to implement this their own way. 
At least, I think the file reader functions should be called from 
getDefaultTimeZoneID, not getDefaultTimeZone(). This way, other 
platforms can avoid them if they want.

- In the TimeZone initializer, we should set the user.timezone system 
property if the user didn't set it.

- Its not good to silently ignore exceptions:

+    catch (IOException ioe)
+      {
+        return null;
+      }
+    finally
+      {
+        try
+          {
+            if (isr != null)
+              isr.close();
+          }
+        catch (IOException ioe)
+          {
+            //ignored
+          }
+      }


Instead we should "code defensively" to avoid throwing exceptions 
wherever possible, especially in code that might run during 
runtime/application startup. Aside from being somewhat slow at the best 
of times, throwing an exception brings a lot of extra data (unwind 
tables) into memory. That increases startup time and increases memory usage.

Ideally IOExceptions should not be ignored unless absolutely necessary, 
for example we should check file.exists() before trying to create a 
stream on it and read from it.

Regards

Bryce


Mark Wielaard wrote:

> Our TimeZone support is not optimal and a little fragile. See the
> TimeZone meta-bug http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16990
>
> This patch tries to solve issues PR17002
> "java.util.TimeZone.getDefault() is broken".
>
> TimeZone.getDefault() has two bugs mapping short timezone names (plus
> GMT-offset) to long/display timezone information.
>
> - The interface between getDefault() and the native getDefaultTimeZoneId() is
> unclear. Which means that the result of getDefaultTimeZoneId() is not always
> mapped to the correct TimeZone.
>
> - getDefaultTimeZoneId() should always return the standard (std) short timezone
> name, never the alternative (dst) Daylight Savings Time name. When it returns a
> dst name the getDefault() mapping from short name to long/display timezone name
> also breaks.
>
> Since using "standard" methods to get at the time zone and gmt offset is
> fragile the patch also introduces some other ways to get at the timezone
> data/name. First (as also done in the past) it tries to use the System
> property "user.timezone", then the environment variable "TZ", then it
> tries to read the /etc/timezone, then it tries to parse the
> /etc/localtime (tz)file and finally, if all else fails, it uses the
> (rewritten) native platform specific getDefaultTimeZoneId() function.
>
> In the end we might just want to adopt the Olson tz data timezone
> package completely as glibc does. Unfortunately neither the tzcode nor
> glibc has any public interface to access all the available data if it is
> available on the system. But the new readtzFile() method might be a
> start to get our own parser for this data.
>
> 2004-08-22  Mark Wielaard  <mark@klomp.org>
>
>       * java/util/TimeZone.java (defaultZone): Try a couple of ways to get
>       a TimeZoneId string and then try to convert that to a TimeZone with
>       getDefaultSystemTimeZone(String).
>       (timezones0): Changed type from Hashtable to HashMap.
>       (timezones): Create HashMap, not Hashtable.
>       (getDefaultTimeZone): New method, rewritten from CNI version.
>       (readTimeZoneFile): New method.
>       (readtzFile): Likewise.
>       (skipFully): Likewise.
>       * java/util/natTimeZone.cc (getSystemTimeZone): Renamed to
>       getDefaultTimeZoneId and rewritten.
>       (getDefaultTimeZoneId): Rewritten in java.
>
> OK to commit?
>
> If this works out on all platforms I like to restructure it to refactor
> out the pieces that should go into a VMTimeZone class so it can be
> adopted by GNU Classpath upstream.
>
> Cheers,
>
> Mark