[ 
https://issues.apache.org/jira/browse/LUCENE-1653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

David Smiley updated LUCENE-1653:
---------------------------------

    Attachment: cleanerDateTools.patch

Applying the attached patch shows the improvements to DateTools.java that I 
think should be done.  All logic that does anything at all is moved to instance 
methods of the inner class Resolution.  I argue this is more object-oriented.

1. In cases where Resolution is an argument to the method, I can simply invoke 
the appropriate call on the Resolution object.  Formerly there was a big branch 
if/else.
2. Instead of "synchronized" being used seemingly everywhere, synchronized is 
used to sync on the object that is not threadsafe, be it a DateFormat or 
Calendar instance.
3. Since different DateFormat and Calendar instances are created 
per-Resolution, there is now less lock contention since threads using different 
resolutions will not use the same locks.
4. The old implementation of timeToString rounded the time before formatting 
it.  That's unnecessary since the format only includes the resolution desired.
5.round() now uses a switch statement that benefits from fall-through (no 
break).

Another debatable improvement is putting the resolution instances into an array 
indexed by format length.  This would mean I could remove big-ish switch in 
lookupResolutionByLength() and avoid the length constants there... but I don't 
think that's a big deal.

> Change DateTools to not create a Calendar in every call to dateToString or 
> timeToString
> ---------------------------------------------------------------------------------------
>
>                 Key: LUCENE-1653
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1653
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Other
>            Reporter: Shai Erera
>            Assignee: Mark Miller
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: cleanerDateTools.patch, LUCENE-1653.patch, 
> LUCENE-1653.patch
>
>
> DateTools creates a Calendar instance on every call to dateToString and 
> timeToString. Specifically:
> # timeToString calls Calendar.getInstance on every call.
> # dateToString calls timeToString(date.getTime()), which then instantiates a 
> new Date(). I think we should change the order of the calls, or not have each 
> call the other.
> # round(), which is called from timeToString (after creating a Calendar 
> instance) creates another (!) Calendar instance ...
> Seems that if we synchronize the methods and create the Calendar instance 
> once (static), it should solve it.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-dev-h...@lucene.apache.org

Reply via email to