dateFromFields slow
vschoettke opened this issue · 10 comments
Elm 0.16 (core) has no way, I know of, to initialize a date except with a string or the timestamp and is also missing date manipulation functionality. So I found your package, which contains all the functionality I need. However the way dateFromFields is converting the date very slow. I have to deal with a lot of Dates and calculations/checks of them so this really hurts the overall application performance.
On my machine initialization of a Date with dateFromFields and year 1975 requires about 5ms and with year 2016 it takes 28ms. Native JS new Date(year, ...) requires always about 0.02ms.
Can you somehow improve the performance of your elm algorithm or maybe add a initialization with native functionality, or should I file a issue on elm-core to provide native date initialization functionality?
I went and checked and yes it gets slower the further from 1970 you get, it does things in a naive fashion which made for minimal code but not great performance in the conversion.
I do believe I can improve the performance a lot (i think an order magnitude is quite possibe). I have some code written but it is still faulty and at least one corner case I have to think about and test further.
Maybe the source from the c++ implementation of mktime here http://opensource.apple.com//source/ntp/ntp-13/ntp/libntp/mktime.c
will help you with some ideas.
On the other hand I'm not sure if it's really worth the effort to reimplement such fundamental function like the native (JS) Date initialization. If Elm will support compilation someday to other targets there is always a similar function like mktime in c available.
I have a new function that is a lot faster in my limited testing here.
I am pretty confident this works identical to the other code I found a great write up of exactly the algorithm i was looking for.
I have put the new version of dateFromFields into the attached file.
Just import this one into your code and call it should be all thats required.
A warning this one does not bound checks the input, so feeding it bad data may well cause incorrect results.
I'd love to have some numbers the performance old and new code.
Just save this file to your project getting rid of the ".txt" extension.
I'll look pulling it into the library soon, I need to think about clamping the inputs to boundary conditions a bit further.
I dug up elm-benchmark this one should be a minimum of 20x times faster, and for years above 2000 its over 1000x times faster.
Nice. I can't really measure a difference between your new implementation and the native new Date(). I converted 100000 years (from year 100 to 100100) and it took on my machine ~49.159ms (+-5ms). I'll not try to compare this with the old implementation ;)
It worked correctly for the month/day combinations I picked but below year 100 it added 1900 to the year.
I don't have any logic to try and convert 0-99 years into 2000 based dates, especially not in the from fields logic. I will look at it when I get home after work again.
Glad to hear its improved things.
My mistake... I tried the new code you attached here with version 3.0.0 before. With the latest Version 4.0.0 it works as expected.
Hmm v4 or previous v3 should not have made a difference.
I am happy to hear its gone, wasn't sure where to look for a problem there :).
Just to verify you arn't getting the +1900 to years below 100 then ?
I've published 4.0.1 with the faster dateFromFields.
It also does the same valid field value clamping as the old one, so havn't lost any safety.
Ok, I rechecked and found that 3.0.0 was working correctly. Then I found out that not your code but the native JS implementation has this behavior. if year is 0-99 it will add 1900... sorry for the mixup
Ahh.. that's a bit scary.
Anyway 4.0.1 onward and upwards...