From 17f22d5021575f36f61e39b4343f3d9c2dcbf39e Mon Sep 17 00:00:00 2001
From: matthew_swift <matthew_swift@localhost>
Date: Fri, 10 Apr 2009 14:15:21 +0000
Subject: [PATCH] Fix issue 3001: Performance bottleneck in GeneralizedTimeSyntax.format

---
 opendj-sdk/opends/src/server/org/opends/server/schema/GeneralizedTimeSyntax.java                             |  159 ++++++++++++++++++--------
 opendj-sdk/opends/src/server/org/opends/server/util/TimeThread.java                                          |   15 -
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/schema/GeneralizedTimeSyntaxTest.java |  136 ++++++++++++++++++++++
 3 files changed, 249 insertions(+), 61 deletions(-)

diff --git a/opendj-sdk/opends/src/server/org/opends/server/schema/GeneralizedTimeSyntax.java b/opendj-sdk/opends/src/server/org/opends/server/schema/GeneralizedTimeSyntax.java
index ad27471..a685a26 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/schema/GeneralizedTimeSyntax.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/schema/GeneralizedTimeSyntax.java
@@ -22,13 +22,13 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2006-2008 Sun Microsystems, Inc.
+ *      Copyright 2006-2009 Sun Microsystems, Inc.
+ *      Portions Copyright 2009 D. J. Hagberg, Millibits Consulting, Inc.
  */
 package org.opends.server.schema;
 
 
 
-import java.text.SimpleDateFormat;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.GregorianCalendar;
@@ -70,19 +70,9 @@
    */
   private static final DebugTracer TRACER = getTracer();
 
-  /**
-   * The lock that will be used to provide threadsafe access to the date
-   * formatter.
-   */
-  private static Object dateFormatLock;
-
-
-
-  /**
-   * The date formatter that will be used to convert dates into generalized time
-   * values.  Note that all interaction with it must be synchronized.
-   */
-  private static SimpleDateFormat dateFormat;
+  // UTC TimeZone is assumed to never change over JVM lifetime
+  private static final TimeZone TIME_ZONE_UTC_OBJ =
+      TimeZone.getTimeZone(TIME_ZONE_UTC);
 
 
 
@@ -97,21 +87,6 @@
 
 
 
-  /*
-   * Create the date formatter that will be used to construct and parse
-   * normalized generalized time values.
-   */
-  static
-  {
-    dateFormat = new SimpleDateFormat(DATE_FORMAT_GENERALIZED_TIME);
-    dateFormat.setLenient(false);
-    dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
-
-    dateFormatLock = new Object();
-  }
-
-
-
   /**
    * Creates a new instance of this syntax.  Note that the only thing that
    * should be done here is to invoke the default constructor for the
@@ -128,6 +103,7 @@
   /**
    * {@inheritDoc}
    */
+  @Override
   public void initializeSyntax(AttributeSyntaxCfg configuration)
          throws ConfigException
   {
@@ -163,6 +139,7 @@
    *
    * @return  The common name for this attribute syntax.
    */
+  @Override
   public String getSyntaxName()
   {
     return SYNTAX_GENERALIZED_TIME_NAME;
@@ -175,6 +152,7 @@
    *
    * @return  The OID for this attribute syntax.
    */
+  @Override
   public String getOID()
   {
     return SYNTAX_GENERALIZED_TIME_OID;
@@ -187,6 +165,7 @@
    *
    * @return  A description for this attribute syntax.
    */
+  @Override
   public String getDescription()
   {
     return SYNTAX_GENERALIZED_TIME_DESCRIPTION;
@@ -202,6 +181,7 @@
    *          attributes with this syntax, or <CODE>null</CODE> if equality
    *          matches will not be allowed for this type by default.
    */
+  @Override
   public EqualityMatchingRule getEqualityMatchingRule()
   {
     return defaultEqualityMatchingRule;
@@ -217,6 +197,7 @@
    *          attributes with this syntax, or <CODE>null</CODE> if ordering
    *          matches will not be allowed for this type by default.
    */
+  @Override
   public OrderingMatchingRule getOrderingMatchingRule()
   {
     return defaultOrderingMatchingRule;
@@ -232,6 +213,7 @@
    *          attributes with this syntax, or <CODE>null</CODE> if substring
    *          matches will not be allowed for this type by default.
    */
+  @Override
   public SubstringMatchingRule getSubstringMatchingRule()
   {
     return defaultSubstringMatchingRule;
@@ -247,6 +229,7 @@
    *          attributes with this syntax, or <CODE>null</CODE> if approximate
    *          matches will not be allowed for this type by default.
    */
+  @Override
   public ApproximateMatchingRule getApproximateMatchingRule()
   {
     // Approximate matching will not be allowed by default.
@@ -267,6 +250,7 @@
    * @return  <CODE>true</CODE> if the provided value is acceptable for use with
    *          this syntax, or <CODE>false</CODE> if not.
    */
+  @Override
   public boolean valueIsAcceptable(ByteSequence value,
                                    MessageBuilder invalidReason)
   {
@@ -293,10 +277,7 @@
    */
   public static String format(Date d)
   {
-    synchronized (dateFormatLock)
-    {
-      return dateFormat.format(d);
-    }
+    return d == null ? null : format(d.getTime());
   }
 
 
@@ -310,12 +291,95 @@
    */
   public static String format(long t)
   {
-    synchronized (dateFormatLock)
-    {
-      return dateFormat.format(new Date(t));
-    }
-  }
+    // Generalized time has the format yyyyMMddHHmmss.SSS'Z'
 
+    // Do this in a thread-safe non-synchronized fashion.
+    // (Simple)DateFormat is neither fast nor thread-safe.
+
+    StringBuilder sb = new StringBuilder(19);
+
+    GregorianCalendar calendar = new GregorianCalendar(TIME_ZONE_UTC_OBJ);
+    calendar.setLenient(false);
+    calendar.setTimeInMillis(t);
+
+    // Format the year yyyy.
+    int n = calendar.get(Calendar.YEAR);
+    if (n < 0)
+    {
+      throw new IllegalArgumentException("Year cannot be < 0:" + n);
+    }
+    else if (n < 10)
+    {
+      sb.append("000");
+    }
+    else if (n < 100)
+    {
+      sb.append("00");
+    }
+    else if (n < 1000)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the month MM.
+    n = calendar.get(Calendar.MONTH) + 1;
+    if (n < 10)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the day dd.
+    n = calendar.get(Calendar.DAY_OF_MONTH);
+    if (n < 10)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the hour HH.
+    n = calendar.get(Calendar.HOUR_OF_DAY);
+    if (n < 10)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the minute mm.
+    n = calendar.get(Calendar.MINUTE);
+    if (n < 10)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the seconds ss.
+    n = calendar.get(Calendar.SECOND);
+    if (n < 10)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the milli-seconds.
+    sb.append('.');
+    n = calendar.get(Calendar.MILLISECOND);
+    if (n < 10)
+    {
+      sb.append("00");
+    }
+    else if (n < 100)
+    {
+      sb.append("0");
+    }
+    sb.append(n);
+
+    // Format the timezone (always Z).
+    sb.append('Z');
+
+    return sb.toString();
+  }
 
 
 
@@ -329,13 +393,7 @@
    */
   public static AttributeValue createGeneralizedTimeValue(long time)
   {
-    String valueString;
-
-    synchronized (dateFormatLock)
-    {
-      valueString = dateFormat.format(new Date(time));
-    }
-
+    String valueString = format(time);
     return AttributeValues.create(ByteString.valueOf(valueString),
         ByteString.valueOf(valueString));
   }
@@ -946,7 +1004,7 @@
           {
             GregorianCalendar calendar = new GregorianCalendar();
             calendar.setLenient(false);
-            calendar.setTimeZone(TimeZone.getTimeZone(TIME_ZONE_UTC));
+            calendar.setTimeZone(TIME_ZONE_UTC_OBJ);
             calendar.set(year, month, day, hour, minute, second);
             calendar.set(Calendar.MILLISECOND, 0);
             return calendar.getTimeInMillis();
@@ -1133,7 +1191,7 @@
           {
             GregorianCalendar calendar = new GregorianCalendar();
             calendar.setLenient(false);
-            calendar.setTimeZone(TimeZone.getTimeZone(TIME_ZONE_UTC));
+            calendar.setTimeZone(TIME_ZONE_UTC_OBJ);
             calendar.set(year, month, day, hour, minute, second);
             calendar.set(Calendar.MILLISECOND, 0);
             return calendar.getTimeInMillis();
@@ -1231,7 +1289,7 @@
           {
             GregorianCalendar calendar = new GregorianCalendar();
             calendar.setLenient(false);
-            calendar.setTimeZone(TimeZone.getTimeZone(TIME_ZONE_UTC));
+            calendar.setTimeZone(TIME_ZONE_UTC_OBJ);
             calendar.set(year, month, day, hour, minute, second);
             calendar.set(Calendar.MILLISECOND, 0);
             return calendar.getTimeInMillis();
@@ -1375,7 +1433,7 @@
                                          message);
           }
 
-          timeZone = TimeZone.getTimeZone(TIME_ZONE_UTC);
+          timeZone = TIME_ZONE_UTC_OBJ;
           break outerLoop;
 
         case '+':
@@ -1590,6 +1648,7 @@
   /**
    * {@inheritDoc}
    */
+  @Override
   public boolean isBinary()
   {
     return false;
diff --git a/opendj-sdk/opends/src/server/org/opends/server/util/TimeThread.java b/opendj-sdk/opends/src/server/org/opends/server/util/TimeThread.java
index 03d9d9b..38bcd0b 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/util/TimeThread.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/util/TimeThread.java
@@ -22,7 +22,7 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2006-2008 Sun Microsystems, Inc.
+ *      Copyright 2006-2009 Sun Microsystems, Inc.
  */
 package org.opends.server.util;
 
@@ -41,6 +41,7 @@
 
 import static org.opends.server.loggers.debug.DebugLogger.*;
 import org.opends.server.loggers.debug.DebugTracer;
+import org.opends.server.schema.GeneralizedTimeSyntax;
 import org.opends.server.types.DebugLogLevel;
 import static org.opends.server.util.ServerConstants.*;
 
@@ -96,9 +97,6 @@
   // The current time in nanoseconds.
   private static volatile long nanoTime;
 
-  // The date formatter that will be used to obtain the generalized time.
-  private static SimpleDateFormat generalizedTimeFormatter;
-
   // The date formatter that will be used to obtain the local timestamp.
   private static SimpleDateFormat localTimestampFormatter;
 
@@ -130,10 +128,6 @@
 
     TimeZone utcTimeZone = TimeZone.getTimeZone(TIME_ZONE_UTC);
 
-    generalizedTimeFormatter =
-         new SimpleDateFormat(DATE_FORMAT_GENERALIZED_TIME);
-    generalizedTimeFormatter.setTimeZone(utcTimeZone);
-
     gmtTimestampFormatter = new SimpleDateFormat(DATE_FORMAT_GMT_TIME);
     gmtTimestampFormatter.setTimeZone(utcTimeZone);
 
@@ -143,7 +137,7 @@
     date            = calendar.getTime();
     time            = date.getTime();
     nanoTime        = System.nanoTime();
-    generalizedTime = generalizedTimeFormatter.format(date);
+    generalizedTime = GeneralizedTimeSyntax.format(date);
     localTimestamp  = localTimestampFormatter.format(date);
     gmtTimestamp    = gmtTimestampFormatter.format(date);
     hourAndMinute   = (calendar.get(Calendar.HOUR_OF_DAY) * 100) +
@@ -158,6 +152,7 @@
    * Operates in a loop, getting the current time and then sleeping briefly
    * before checking again.
    */
+  @Override
   public void run()
   {
     while (true)
@@ -168,7 +163,7 @@
         date            = calendar.getTime();
         time            = date.getTime();
         nanoTime        = System.nanoTime();
-        generalizedTime = generalizedTimeFormatter.format(date);
+        generalizedTime = GeneralizedTimeSyntax.format(date);
         localTimestamp  = localTimestampFormatter.format(date);
         gmtTimestamp    = gmtTimestampFormatter.format(date);
         hourAndMinute   = (calendar.get(Calendar.HOUR_OF_DAY) * 100) +
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/schema/GeneralizedTimeSyntaxTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/schema/GeneralizedTimeSyntaxTest.java
index 3490169..cb5f1ac 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/schema/GeneralizedTimeSyntaxTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/schema/GeneralizedTimeSyntaxTest.java
@@ -22,12 +22,21 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2006-2008 Sun Microsystems, Inc.
+ *      Copyright 2006-2009 Sun Microsystems, Inc.
  */
 package org.opends.server.schema;
 
+import static org.opends.server.util.ServerConstants.*;
+
+import java.util.Calendar;
+import java.util.Date;
+import java.util.GregorianCalendar;
+import java.util.TimeZone;
+
 import org.opends.server.api.AttributeSyntax;
+import org.testng.Assert;
 import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
 
 /**
  * Test the GeneralizedTimeSyntax.
@@ -80,4 +89,129 @@
     };
   }
 
+
+
+  /**
+   * Create data for format(...) tests.
+   *
+   * @return Returns test data.
+   */
+  @DataProvider(name="createFormatData")
+  public Object[][] createFormatData()
+  {
+    return new Object [][] {
+        // Note that Calendar months run from 0-11,
+        // and that there was no such year as year 0 (1 BC -> 1 AD).
+        {   1,  0,  1,  0,  0,  0,   0, "00010101000000.000Z"},
+        {   9,  0,  1,  0,  0,  0,   0, "00090101000000.000Z"},
+        {  10,  0,  1,  0,  0,  0,   0, "00100101000000.000Z"},
+        {  99,  0,  1,  0,  0,  0,   0, "00990101000000.000Z"},
+        { 100,  0,  1,  0,  0,  0,   0, "01000101000000.000Z"},
+        { 999,  0,  1,  0,  0,  0,   0, "09990101000000.000Z"},
+        {1000,  0,  1,  0,  0,  0,   0, "10000101000000.000Z"},
+        {2000,  0,  1,  0,  0,  0,   0, "20000101000000.000Z"},
+        {2099,  0,  1,  0,  0,  0,   0, "20990101000000.000Z"},
+        {2000,  8,  1,  0,  0,  0,   0, "20000901000000.000Z"},
+        {2000,  9,  1,  0,  0,  0,   0, "20001001000000.000Z"},
+        {2000, 10,  1,  0,  0,  0,   0, "20001101000000.000Z"},
+        {2000, 11,  1,  0,  0,  0,   0, "20001201000000.000Z"},
+        {2000,  0,  9,  0,  0,  0,   0, "20000109000000.000Z"},
+        {2000,  0, 10,  0,  0,  0,   0, "20000110000000.000Z"},
+        {2000,  0, 19,  0,  0,  0,   0, "20000119000000.000Z"},
+        {2000,  0, 20,  0,  0,  0,   0, "20000120000000.000Z"},
+        {2000,  0, 29,  0,  0,  0,   0, "20000129000000.000Z"},
+        {2000,  0, 30,  0,  0,  0,   0, "20000130000000.000Z"},
+        {2000,  0, 31,  0,  0,  0,   0, "20000131000000.000Z"},
+        {2000,  0,  1,  9,  0,  0,   0, "20000101090000.000Z"},
+        {2000,  0,  1, 10,  0,  0,   0, "20000101100000.000Z"},
+        {2000,  0,  1, 19,  0,  0,   0, "20000101190000.000Z"},
+        {2000,  0,  1, 20,  0,  0,   0, "20000101200000.000Z"},
+        {2000,  0,  1, 23,  0,  0,   0, "20000101230000.000Z"},
+        {2000,  0,  1,  0,  9,  0,   0, "20000101000900.000Z"},
+        {2000,  0,  1,  0, 10,  0,   0, "20000101001000.000Z"},
+        {2000,  0,  1,  0, 59,  0,   0, "20000101005900.000Z"},
+        {2000,  0,  1,  0,  0,  9,   0, "20000101000009.000Z"},
+        {2000,  0,  1,  0,  0, 10,   0, "20000101000010.000Z"},
+        {2000,  0,  1,  0,  0, 59,   0, "20000101000059.000Z"},
+        {2000,  0,  1,  0,  0,  0,   9, "20000101000000.009Z"},
+        {2000,  0,  1,  0,  0,  0,  10, "20000101000000.010Z"},
+        {2000,  0,  1,  0,  0,  0,  99, "20000101000000.099Z"},
+        {2000,  0,  1,  0,  0,  0, 100, "20000101000000.100Z"},
+        {2000,  0,  1,  0,  0,  0, 999, "20000101000000.999Z"},
+    };
+  }
+
+
+
+  /**
+   * Tests {@link GeneralizedTimeSyntax#format(long)}.
+   *
+   * @param yyyy
+   *          The year.
+   * @param MM
+   *          The month.
+   * @param dd
+   *          The day.
+   * @param HH
+   *          The hour.
+   * @param mm
+   *          The minute.
+   * @param ss
+   *          The second.
+   * @param SSS
+   *          The milli-seconds.
+   * @param expected
+   *          The expected generalized time formatted string.
+   * @throws Exception
+   *           If an unexpected error occurred.
+   */
+  @Test(dataProvider="createFormatData")
+  public void testFormatLong(int yyyy, int MM, int dd, int HH, int mm,
+      int ss, int SSS, String expected) throws Exception
+  {
+    Calendar calendar =
+        new GregorianCalendar(TimeZone.getTimeZone(TIME_ZONE_UTC));
+    calendar.set(yyyy, MM, dd, HH, mm, ss);
+    calendar.set(Calendar.MILLISECOND, SSS);
+    long time = calendar.getTimeInMillis();
+    String actual = GeneralizedTimeSyntax.format(time);
+    Assert.assertEquals(actual, expected);
+  }
+
+
+
+  /**
+   * Tests {@link GeneralizedTimeSyntax#format(Date)}.
+   *
+   * @param yyyy
+   *          The year.
+   * @param MM
+   *          The month.
+   * @param dd
+   *          The day.
+   * @param HH
+   *          The hour.
+   * @param mm
+   *          The minute.
+   * @param ss
+   *          The second.
+   * @param SSS
+   *          The milli-seconds.
+   * @param expected
+   *          The expected generalized time formatted string.
+   * @throws Exception
+   *           If an unexpected error occurred.
+   */
+  @Test(dataProvider="createFormatData")
+  public void testFormatDate(int yyyy, int MM, int dd, int HH, int mm,
+      int ss, int SSS, String expected) throws Exception
+  {
+    Calendar calendar =
+        new GregorianCalendar(TimeZone.getTimeZone(TIME_ZONE_UTC));
+    calendar.set(yyyy, MM, dd, HH, mm, ss);
+    calendar.set(Calendar.MILLISECOND, SSS);
+    Date time = new Date(calendar.getTimeInMillis());
+    String actual = GeneralizedTimeSyntax.format(time);
+    Assert.assertEquals(actual, expected);
+  }
 }

--
Gitblit v1.10.0