andrewmcveigh / cljs-time

A clj-time inspired date library for clojurescript.
342 stars 57 forks source link

don't construct ex-info if we don't need it #86

Closed johnswanson closed 7 years ago

johnswanson commented 7 years ago

Profiling an application that's parsing a thousand or so dates at a time, I found that 75% of the time was spent constructing ex-infos (specifically, it looks like getting the stack is a very expensive operation).

example

In most parsing cases, we probably won't actually be throwing any error, so instead of wastefully constructing the ex-info every time, we can just construct it when it's actually thrown.

I did this by just throwing the ex-info generation into a separate function, to avoid code repetition.

andrewmcveigh commented 7 years ago

Thanks for catching this!

I think @danielcompton has a point. It's possibly better to defn- this, or at least locally let a fn outside the loop, but I'll happily merge as is.

johnswanson commented 7 years ago

Thanks @andrewmcveigh and @danielcompton, sorry for missing the comment re: creating the function within the loop until now! That makes sense to me, I'd be happy to create another PR doing the defn- instead.