harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tony Wu" <wuyue...@gmail.com>
Subject Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Date Tue, 02 Sep 2008 00:57:23 GMT
Hi, Nathan
The format pattern is timezone independent but the value might be
different. For instance, the time 2008-09-01 08:00:00 in GMT should be
2008-09-01 16:00:00 in CTT(GMT+8).


On 9/2/08, Nathan Beyer <ndbeyer@apache.org> wrote:
> Since the classes are supposed to be TimeZone-independant, couldn't
> all of the TimeZone code be removed?
>
> -Nathan
>
> On Mon, Sep 1, 2008 at 12:19 AM, Tony Wu <wuyuehao@gmail.com> wrote:
> > I reviewed these 3 tests. It's true that locales other than GMT have
> > been covered in the TimeTest and TimeStampTest but not in DateTest.
> > And the comments in the testToString is confusing ...
> > Also some testcases in TimestampTest have dependency to each other.
> > Since other locales have been covered by testToString, I just
> > explicity set the default locale to GMT at the beginning of these
> > testcases.
> >
> > I've corrected them at r690847.
> >
> > On Sat, Aug 30, 2008 at 10:22 AM, Nathan Beyer <ndbeyer@apache.org> wrote:
> >> But, the tests are invalid, correct? At a minimum, all of the TimeZone
> >> code in the tests is unnecessary.
> >>
> >> I'm not trying to insinuate or infer that the patch is wrong. I just
> >> like to look at each code change in a larger context. In this case,
> >> the tests are still passing, but upon closer inspection of the tests,
> >> they seem to be invalid or, as mentioned above, they have unnecessary
> >> comments and seemingly invalid comments.
> >>
> >> -Nathan
> >>
> >> On Thu, Aug 28, 2008 at 9:46 PM, Tony Wu <wuyuehao@gmail.com> wrote:
> >>> I think so. The format has been explicitly specified by spec, such as
> >>> yyyy-mm-dd hh:mm:ss.fffffffff
> >>>
> >>> Thus it should be locale independent and this patch is valid.
> >>>
> >>> On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <ndbeyer@apache.org>
wrote:
> >>>> I looked at the test in further detail and they seem okay. The tests
> >>>> seem odd though, as they set TimeZone values and mention things like
> >>>> TimeZone affecting the output. That shouldn't be the case though,
> >>>> correct? The formats are all in UTC/GMT, correct?
> >>>>
> >>>> -Nathan
> >>>>
> >>>> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <sean.xx.qiu@gmail.com>
wrote:
> >>>>> As Regis said, there exist tests for these toString method, which,
> >>>>> IMHO,  are covering this modfication.
> >>>>> Shall we add more tests?
> >>>>>
> >>>>> 2008/8/28 Nathan Beyer <nbeyer@gmail.com>:
> >>>>>> I was referring to a functional test, not a performance test.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 8/27/08, Sean Qiu <sean.xx.qiu@gmail.com> wrote:
> >>>>>>> Here is a small test from Regis:
> >>>>>>>
> >>>>>>> ----------
> >>>>>>>  int count = 5000;
> >>>>>>>
> >>>>>>> for (int i = 0; i < count; ++i) { new Date(new
> >>>>>>> java.util.Date().getTime()).toString(); }
> >>>>>>>
> >>>>>>> Date date = new Date(new java.util.Date().getTime());
> >>>>>>> long start = System.currentTimeMillis();
> >>>>>>>
> >>>>>>> for (int i = 0; i < count; ++i) { date.toString(); }
> >>>>>>> long end = System.currentTimeMillis();
> >>>>>>> System.out.println("Invoke Date.toString() " + count + "
times, cost:
> >>>>>>> " + (end - start));
> >>>>>>>
> >>>>>>> Time time = new Time(new java.util.Date().getTime());
> >>>>>>> start = System.currentTimeMillis();
> >>>>>>> for (int i = 0; i < count; ++i) { time.toString(); }
> >>>>>>> end = System.currentTimeMillis();
> >>>>>>> System.out.println("Invoke Time.toString() " + count + "
times, cost:
> >>>>>>> " + (end - start));
> >>>>>>>
> >>>>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
> >>>>>>> start = System.currentTimeMillis();
> >>>>>>> for (int i = 0; i < count; ++i) { timestamp.toString();
}
> >>>>>>> end = System.currentTimeMillis();
> >>>>>>> System.out.println("Invoke Timestamp.toString() " + count
+ " times,
> >>>>>>> cost: " + (end - start));
> >>>>>>> -------------
> >>>>>>>
> >>>>>>> the first loop, give time for jvm to warm up
> >>>>>>>
> >>>>>>> Below data compare the two implementations:
> >>>>>>>
> >>>>>>> before the patch:
> >>>>>>> Invoke Date.toString() 5000 times, cost: 6757
> >>>>>>> Invoke Time.toString() 5000 times, cost: 7699
> >>>>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
> >>>>>>>
> >>>>>>> after the patch:
> >>>>>>> Invoke Date.toString() 5000 times, cost: 84
> >>>>>>> Invoke Time.toString() 5000 times, cost: 95
> >>>>>>> Invoke Timestamp.toString() 5000 times, cost: 272
> >>>>>>>
> >>>>>>>
> >>>>>>> We can gain obvious improvement.
> >>>>>>>
> >>>>>>> 2008/8/28 Nathan Beyer <ndbeyer@apache.org>:
> >>>>>>>> Are there any associated test cases with this change?
On a quick
> >>>>>>>> cursory look, I didn't see any existing tests. Did I
miss them? If
> >>>>>>>> not, we need to start requiring some test cases for
"simple
> >>>>>>>> improvements" like this to continue functional correctness.
> >>>>>>>>
> >>>>>>>> -Nathan
> >>>>>>>>
> >>>>>>>> On Tue, Aug 26, 2008 at 3:59 AM,  <qiuxx@apache.org>
wrote:
> >>>>>>>>> Author: qiuxx
> >>>>>>>>> Date: Tue Aug 26 01:59:50 2008
> >>>>>>>>> New Revision: 689001
> >>>>>>>>>
> >>>>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
> >>>>>>>>> Log:
> >>>>>>>>> Apply for HARMONY-5958,([classlib][sql][performance]
- improve
> >>>>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>>
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>>
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>>
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>> URL:
> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
> >>>>>>>>> ==============================================================================
> >>>>>>>>> ---
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>> (original)
> >>>>>>>>> +++
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>> Tue Aug 26 01:59:50 2008
> >>>>>>>>> @@ -17,8 +17,6 @@
> >>>>>>>>>
> >>>>>>>>>  package java.sql;
> >>>>>>>>>
> >>>>>>>>> -import java.text.SimpleDateFormat;
> >>>>>>>>> -
> >>>>>>>>>  /**
> >>>>>>>>>  * A Date class which can consume and produce dates
in SQL Date format.
> >>>>>>>>>  * <p>
> >>>>>>>>> @@ -175,8 +173,28 @@
> >>>>>>>>>      */
> >>>>>>>>>     @Override
> >>>>>>>>>     public String toString() {
> >>>>>>>>> -        SimpleDateFormat dateFormat = new
> >>>>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
> >>>>>>>>> -        return dateFormat.format(this);
> >>>>>>>>> +        StringBuilder sb = new StringBuilder(10);
> >>>>>>>>> +
> >>>>>>>>> +        format((getYear() + 1900), 4, sb);
> >>>>>>>>> +        sb.append('-');
> >>>>>>>>> +        format((getMonth() + 1), 2, sb);
> >>>>>>>>> +        sb.append('-');
> >>>>>>>>> +        format(getDate(), 2, sb);
> >>>>>>>>> +
> >>>>>>>>> +        return sb.toString();
> >>>>>>>>> +    }
> >>>>>>>>> +
> >>>>>>>>> +    private static final String PADDING = "0000";
 //$NON-NLS-1$
> >>>>>>>>> +
> >>>>>>>>> +    /*
> >>>>>>>>> +    * Private method to format the time
> >>>>>>>>> +    */
> >>>>>>>>> +    private void format(int date, int digits, StringBuilder
sb) {
> >>>>>>>>> +        String str = String.valueOf(date);
> >>>>>>>>> +        if (digits - str.length() > 0) {
> >>>>>>>>> +            sb.append(PADDING.substring(0, digits
- str.length()));
> >>>>>>>>> +        }
> >>>>>>>>> +        sb.append(str);
> >>>>>>>>>     }
> >>>>>>>>>
> >>>>>>>>>     /**
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>> URL:
> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
> >>>>>>>>> ==============================================================================
> >>>>>>>>> ---
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>> (original)
> >>>>>>>>> +++
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>> Tue Aug 26 01:59:50 2008
> >>>>>>>>> @@ -17,7 +17,6 @@
> >>>>>>>>>
> >>>>>>>>>  package java.sql;
> >>>>>>>>>
> >>>>>>>>> -import java.text.SimpleDateFormat;
> >>>>>>>>>  import java.util.Date;
> >>>>>>>>>
> >>>>>>>>>  /**
> >>>>>>>>> @@ -180,8 +179,28 @@
> >>>>>>>>>      */
> >>>>>>>>>     @Override
> >>>>>>>>>     public String toString() {
> >>>>>>>>> -        SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
> >>>>>>>>> //$NON-NLS-1$
> >>>>>>>>> -        return dateFormat.format(this);
> >>>>>>>>> +        StringBuilder sb = new StringBuilder(8);
> >>>>>>>>> +
> >>>>>>>>> +        format(getHours(), 2, sb);
> >>>>>>>>> +        sb.append(':');
> >>>>>>>>> +        format(getMinutes(), 2, sb);
> >>>>>>>>> +        sb.append(':');
> >>>>>>>>> +        format(getSeconds(), 2, sb);
> >>>>>>>>> +
> >>>>>>>>> +        return sb.toString();
> >>>>>>>>> +    }
> >>>>>>>>> +
> >>>>>>>>> +    private static final String PADDING = "00";
 //$NON-NLS-1$
> >>>>>>>>> +
> >>>>>>>>> +    /*
> >>>>>>>>> +    * Private method to format the time
> >>>>>>>>> +    */
> >>>>>>>>> +    private void format(int date, int digits, StringBuilder
sb) {
> >>>>>>>>> +        String str = String.valueOf(date);
> >>>>>>>>> +        if (digits - str.length() > 0) {
> >>>>>>>>> +            sb.append(PADDING.substring(0, digits
- str.length()));
> >>>>>>>>> +        }
> >>>>>>>>> +        sb.append(str);
> >>>>>>>>>     }
> >>>>>>>>>
> >>>>>>>>>     /**
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>> URL:
> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
> >>>>>>>>> ==============================================================================
> >>>>>>>>> ---
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>> (original)
> >>>>>>>>> +++
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>> Tue Aug 26 01:59:50 2008
> >>>>>>>>> @@ -309,62 +309,43 @@
> >>>>>>>>>     @SuppressWarnings("deprecation")
> >>>>>>>>>     @Override
> >>>>>>>>>     public String toString() {
> >>>>>>>>> -        /*
> >>>>>>>>> -         * Use a DecimalFormat to lay out the nanosecond
value as a
> >>>>>>>>> simple
> >>>>>>>>> -         * string of 9 integers, with leading Zeros
> >>>>>>>>> -         */
> >>>>>>>>> -        DecimalFormat decimalFormat = new DecimalFormat("0");
> >>>>>>>>> //$NON-NLS-1$
> >>>>>>>>> -        decimalFormat.setMinimumIntegerDigits(9);
> >>>>>>>>> -        decimalFormat.setMaximumIntegerDigits(9);
> >>>>>>>>> -        String theNanos = decimalFormat.format(nanos);
> >>>>>>>>> -        theNanos = stripTrailingZeros(theNanos);
> >>>>>>>>> -
> >>>>>>>>> -        String year = format((getYear() + 1900),
4);
> >>>>>>>>> -        String month = format((getMonth() + 1),
2);
> >>>>>>>>> -        String date = format(getDate(), 2);
> >>>>>>>>> -        String hours = format(getHours(), 2);
> >>>>>>>>> -        String minutes = format(getMinutes(), 2);
> >>>>>>>>> -        String seconds = format(getSeconds(), 2);
> >>>>>>>>> -
> >>>>>>>>> -        return year + '-' + month + '-' + date
+ ' ' + hours + ':' +
> >>>>>>>>> minutes
> >>>>>>>>> -                + ':' + seconds + '.' + theNanos;
> >>>>>>>>> -    }
> >>>>>>>>> +        StringBuilder sb = new StringBuilder(29);
> >>>>>>>>>
> >>>>>>>>> -    /*
> >>>>>>>>> -     * Private method to format the time
> >>>>>>>>> -     */
> >>>>>>>>> -    private String format(int date, int digits)
{
> >>>>>>>>> -        StringBuilder dateStringBuffer = new
> >>>>>>>>> StringBuilder(String.valueOf(date));
> >>>>>>>>> -        while (dateStringBuffer.length() < digits)
{
> >>>>>>>>> -            dateStringBuffer = dateStringBuffer.insert(0,
'0');
> >>>>>>>>> +        format((getYear() + 1900), 4, sb);
> >>>>>>>>> +        sb.append('-');
> >>>>>>>>> +        format((getMonth() + 1), 2, sb);
> >>>>>>>>> +        sb.append('-');
> >>>>>>>>> +        format(getDate(), 2, sb);
> >>>>>>>>> +        sb.append(' ');
> >>>>>>>>> +        format(getHours(), 2, sb);
> >>>>>>>>> +        sb.append(':');
> >>>>>>>>> +        format(getMinutes(), 2, sb);
> >>>>>>>>> +        sb.append(':');
> >>>>>>>>> +        format(getSeconds(), 2, sb);
> >>>>>>>>> +        sb.append('.');
> >>>>>>>>> +        if (nanos == 0) {
> >>>>>>>>> +            sb.append('0');
> >>>>>>>>> +        } else {
> >>>>>>>>> +            format(nanos, 9, sb);
> >>>>>>>>> +            while (sb.charAt(sb.length() - 1) ==
'0') {
> >>>>>>>>> +                sb.setLength(sb.length() - 1);
> >>>>>>>>> +            }
> >>>>>>>>>         }
> >>>>>>>>> -        return dateStringBuffer.toString();
> >>>>>>>>> +
> >>>>>>>>> +        return sb.toString();
> >>>>>>>>>     }
> >>>>>>>>>
> >>>>>>>>> -    /*
> >>>>>>>>> -     * Private method to strip trailing '0' characters
from a string.
> >>>>>>>>> @param
> >>>>>>>>> -     * inputString the starting string @return
a string with the
> >>>>>>>>> trailing zeros
> >>>>>>>>> -     * stripped - will leave a single 0 at the
beginning of the string
> >>>>>>>>> -     */
> >>>>>>>>> -    private String stripTrailingZeros(String inputString)
{
> >>>>>>>>> -        String finalString;
> >>>>>>>>> +    private static final String PADDING = "000000000";
 //$NON-NLS-1$
> >>>>>>>>>
> >>>>>>>>> -        int i;
> >>>>>>>>> -        for (i = inputString.length(); i > 0;
i--) {
> >>>>>>>>> -            if (inputString.charAt(i - 1) != '0')
{
> >>>>>>>>> -                break;
> >>>>>>>>> -            }
> >>>>>>>>> -            /*
> >>>>>>>>> -             * If the string has a 0 as its first
character, return a
> >>>>>>>>> string
> >>>>>>>>> -             * with a single '0'
> >>>>>>>>> -             */
> >>>>>>>>> -            if (i == 1) {
> >>>>>>>>> -                return "0"; //$NON-NLS-1$
> >>>>>>>>> -            }
> >>>>>>>>> +    /*
> >>>>>>>>> +    * Private method to format the time
> >>>>>>>>> +    */
> >>>>>>>>> +    private void format(int date, int digits, StringBuilder
sb) {
> >>>>>>>>> +        String str = String.valueOf(date);
> >>>>>>>>> +        if (digits - str.length() > 0) {
> >>>>>>>>> +            sb.append(PADDING.substring(0, digits
- str.length()));
> >>>>>>>>>         }
> >>>>>>>>> -
> >>>>>>>>> -        finalString = inputString.substring(0,
i);
> >>>>>>>>> -        return finalString;
> >>>>>>>>> +        sb.append(str);
> >>>>>>>>>     }
> >>>>>>>>>
> >>>>>>>>>     /**
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Best Regards
> >>>>>>> Sean, Xiao Xia Qiu
> >>>>>>>
> >>>>>>> China Software Development Lab, IBM
> >>>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Sent from Gmail for mobile | mobile.google.com
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Best Regards
> >>>>> Sean, Xiao Xia Qiu
> >>>>>
> >>>>> China Software Development Lab, IBM
> >>>>>
> >>>>
> >>>
> >>>
> >>>
> >>> --
> >>> Tony Wu
> >>> China Software Development Lab, IBM
> >>>
> >>
> >
> >
> >
> > --
> > Tony Wu
> > China Software Development Lab, IBM
> >
>


-- 
Tony Wu
China Software Development Lab, IBM

Mime
View raw message