Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      see parent task for background

        Activity

        Hide
        Hoss Man added a comment -

        Committed revision 1171691.

        working on backport to 3x

        Show
        Hoss Man added a comment - Committed revision 1171691. working on backport to 3x
        Hide
        Hoss Man added a comment -

        Committed revision 1171704.

        trunk fixup of an unneccessary change i noticed when working on the backport

        Show
        Hoss Man added a comment - Committed revision 1171704. trunk fixup of an unneccessary change i noticed when working on the backport
        Hide
        Hoss Man added a comment -

        Committed revision 1171739. - stupid jdoc mistake
        Committed revision 1171741. - merge everything to 3x

        Show
        Hoss Man added a comment - Committed revision 1171739. - stupid jdoc mistake Committed revision 1171741. - merge everything to 3x
        Hide
        Yonik Seeley added a comment -

        If the only issue was years less than 1000, we might want to consider bringing back the original hand-rolled formatting code and fixing it.
        A quick performance test shows it's over 4 times as fast.

        Using the current code in DateField, I only got about 25K formats/sec. To put that into perspective, formatting 100 dates takes 4ms - a non -trivial amount of time.

        Show
        Yonik Seeley added a comment - If the only issue was years less than 1000, we might want to consider bringing back the original hand-rolled formatting code and fixing it. A quick performance test shows it's over 4 times as fast. Using the current code in DateField, I only got about 25K formats/sec. To put that into perspective, formatting 100 dates takes 4ms - a non -trivial amount of time.
        Hide
        Hoss Man added a comment -

        1) The original code was more broken then what we have now, so saying it's 4 times as fast is kind of meaningless.

        2) The only issue is not just (positive) years less then 1000, as noted in the parent issue, there is confusion about how to handle years less then 0001 because of ambiguity in xml dateTime format spec about year "0"

        ...as i said in the parent issue, unless someone has a better idea, i'm open to moving towards (correct) hand rolled code – but we have to figure out what is "correct" as far as year 0. This particular child issue was just to move forward with something that worked for year 0001 and above.

        Show
        Hoss Man added a comment - 1) The original code was more broken then what we have now, so saying it's 4 times as fast is kind of meaningless. 2) The only issue is not just (positive) years less then 1000, as noted in the parent issue, there is confusion about how to handle years less then 0001 because of ambiguity in xml dateTime format spec about year "0" ...as i said in the parent issue, unless someone has a better idea, i'm open to moving towards (correct) hand rolled code – but we have to figure out what is "correct" as far as year 0. This particular child issue was just to move forward with something that worked for year 0001 and above.
        Hide
        Simon Willnauer added a comment -

        I don't know if that will help but maybe we should look at http://joda-time.sourceforge.net it seems they worked around the perf issues with the java impl. and its ASL-2 maybe we can include or get some code from them? - ignore me if I'm completely off here

        Show
        Simon Willnauer added a comment - I don't know if that will help but maybe we should look at http://joda-time.sourceforge.net it seems they worked around the perf issues with the java impl. and its ASL-2 maybe we can include or get some code from them? - ignore me if I'm completely off here
        Hide
        Hoss Man added a comment -

        Simon: see parent issue with link to JodaTime thread where i asked about using JodaTime to deal with some of the issues faced by DateField and the XML dateTime spec (including stub code using JodaTime) and got a response that was not very promising.

        Show
        Hoss Man added a comment - Simon: see parent issue with link to JodaTime thread where i asked about using JodaTime to deal with some of the issues faced by DateField and the XML dateTime spec (including stub code using JodaTime) and got a response that was not very promising.
        Hide
        Yonik Seeley added a comment -

        1) The original code was more broken then what we have now, so saying it's 4 times as fast is kind of meaningless.

        Seems relevant that if the only difference in behavior between the old code and the new code is years < 1000, then I could fix the old code and get a 4x speedup.

        Show
        Yonik Seeley added a comment - 1) The original code was more broken then what we have now, so saying it's 4 times as fast is kind of meaningless. Seems relevant that if the only difference in behavior between the old code and the new code is years < 1000, then I could fix the old code and get a 4x speedup.
        Hide
        Hoss Man added a comment -

        I guess i don't understand what exactly you mean by "original".

        The original circa Solr 1.1 hand rolled code for dealing with dates had all sorts of bugs, fixed in various ways over the years – this particular patch fixed two bugs in DateField (related to years 0001-1000 and millis on years before the epoch) and corrected the fact that TextResponseWriter was erroneously not using the well tested formatting code in DateField – so it was still susceptible to some of those old bugs. (do i have the full list of those bugs handy: no, but if you're interested you can search Jira for them)

        If you want to re-write this to be hand rolled go ahead, but please do so in the DateField code (and don't just revert the changes to TextResponseWriter) since tests directly against DateField are where most of the Unit testing of parsing/formatting esoteric dates live – if you keep all those tests happy (w/o writting yet another new set of parse/format methods) then i'll happily welcome whatever high-performance hand rolled solution you have.

        Show
        Hoss Man added a comment - I guess i don't understand what exactly you mean by "original". The original circa Solr 1.1 hand rolled code for dealing with dates had all sorts of bugs, fixed in various ways over the years – this particular patch fixed two bugs in DateField (related to years 0001-1000 and millis on years before the epoch) and corrected the fact that TextResponseWriter was erroneously not using the well tested formatting code in DateField – so it was still susceptible to some of those old bugs. (do i have the full list of those bugs handy: no, but if you're interested you can search Jira for them) If you want to re-write this to be hand rolled go ahead, but please do so in the DateField code (and don't just revert the changes to TextResponseWriter) since tests directly against DateField are where most of the Unit testing of parsing/formatting esoteric dates live – if you keep all those tests happy (w/o writting yet another new set of parse/format methods) then i'll happily welcome whatever high-performance hand rolled solution you have.
        Hide
        Uwe Schindler added a comment -

        Bulk close after 3.5 is released

        Show
        Uwe Schindler added a comment - Bulk close after 3.5 is released

          People

          • Assignee:
            Hoss Man
            Reporter:
            Hoss Man
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development