coerce/from-string is a bit too naive (quite slow)

[bpicolo]

Looks like all the exception handlers that get run end up making from-string pretty inefficient. It probably makes sense why this happens, but maybe should be documented as a trip-up. Not sure if my alternative is a particularly sane version (probably isn't, just a naive first attempt that happens to fit my use case). It could probably at least optimize to hit obvious types first (these are isoformat dates acting slow). Hit this because parsing just ~10-15 dates was making my page hundreds of times slower than expected.

cljs.user=> (simple-benchmark [time "2016-09-13T12:00:00"] (cljs-time.coerce/from-string time) 100)
[time "2016-09-13T12:00:00"], (cljs-time.coerce/from-string time), 100 runs, 1467 msecs
nil
cljs.user=> (simple-benchmark [time "2016-09-13T12:00:00"] (cljs-time.coerce/from-long (.parse js/Date time)) 100)
[time "2016-09-13T12:00:00"], (cljs-time.coerce/from-long (.parse js/Date time)), 100 runs, 0 msecs
nil

[danielcompton]

Parsing dates with Date.parse() isn't recommended by MDN (I know this wasn't really your suggestion, I was just mentioning it as an aside on parsing).

It is not recommended to use Date.parse as until ES5, parsing of strings was entirely implementation dependent. There are still many differences in how different hosts parse date strings, therefore date strings should be manually parsed (a library can help if many different formats are to be accommodated).

It's probably worth documenting that this is a slow method, but it is always going to be inherently slow to parse an arbitrary date format. You could conceivably speed it up by sorting the date formats or trying to do pre matching on regexes, or a bunch of other things, but I'm not sure if it's worth it. If you know up front which format you are going to be parsing, then cljs-time.format is a much safer and faster option.

[bpicolo]

@danielcompton Yep! Agreed there, was just an example. I think part of it is that it seems slow even considering running through n-different formats. (Which is something like 40?) tens-of-ms scale per date is pretty out there. The clojurescript exception handling might make it slower than expected (That's what the js profile suggested, anyway)

[andrewmcveigh]

TBH I'm not really motivated to "fix" this. It's present because the function is present in clj-time, but I don't feel that the function is particularly useful in production software. It's result could be dependent on the "ordering" of a hash-map, depending on which format the date-string is in, so it's not exactly reliable either.

I'd be willing to accept a patch that improved things, provided it didn't add complexity to other parts of the library.