-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Locked into UTCTime/UTCTime is slow #1082
base: master
Are you sure you want to change the base?
Conversation
It's not lame for there to be a built-in That's not locking you in. There are many alternative time libraries with various trade-offs. There is nothing stopping you from writing a I don't believe there is any reason that the |
It's my read of the code/my experience using it that any |
Updated: The benchmarks for UTCTime are a lot closer when you turn off profiling and compile with -O2:
vs chronos:
|
Your custom persistent types can be marshaled to and from the DB however you want, using If you want a custom instance for But for that you still need to parse and render your integers represented as text. Using If you play around with these approaches and benchmark them, you might come up with a really great PR to improve the performance of |
@yitz-zoomin I don't think that's the case. You're saying you would expect this code to work, right?
But instead that code will fail with the error:
Tbc, PersistDbSpecific and SqlOther are great and I use them all the time for things like UUIDs. It's just in this case, timestamps are deserialized to |
I know that @jessekempf also found that parsing UTCTime was a huge drain. Parsing into a Jesse do you mind posting your notes here? Slack ate them and I can't remember what happened :( |
This honestly feels like the perfect use-case for the EDIT: I guess what I'm saying is that I honestly wouldn't be surprised if there was a measurable speed gain from using the |
@parsonsmatt, @MaxGabriel: Ultimately any dense time representation is going to look like a 128-bit number. The overhead for that will be negligible in comparison to the fact that The other problem is that the |
|
(This is really an issue with some supporting code attached, not a true PR)
At work we've got an admin page that returns all of the companies that use our product and some associated data about them. This page has been taking longer and longer to load, and after profiling we found 15% of the time was spent in deserializing
UTCTime
s from the database.My coworker @chessai recommended I check out
chronos
, which Layer 3 and some others use for much faster time parsing.Unfortunately, I think Persistent locks you into using
UTCTime
with Postgres, based on these lines:persistent/persistent-postgresql/Database/Persist/Postgresql.hs
Lines 579 to 580 in 6539d0c
(I could be wrong about this, but it seems that way)
To test out Chronos, I modified persistent-postgresql to return
PersistByteString
for timestamps and made a benchmark. I enabled profiling when running the benchmarks and the generated.prof
files and flamegraphs (generated by the sick https://github.com/fpco/ghc-prof-flamegraph) are in here.This benchmark tested selecting 10,000 rows, each with two
Text
fields and twoUTCTime
fields. UsingUTCTime
, I got:Using
OffsetDatetime
from Chronos, I got:This is a fairly dramatic difference. Because of how common time fields are (especially with common patterns like
createdAt
andupdatedAt
metadata fields), deserializing timestamps can be quite a drain, especially when joining multiple tables with Esqueleto.For context, this is my understanding of why
chronos
is faster:UTCTime
is represented as anInteger
of days since1858-11-17
, plus a number of Picoseconds (10^-12 seconds) (represented by [Pico](https://www.stackage.org/haddock/lts-15.11/base-4.13.0.0/Data-Fixed.html#t:Pico from Data.Fixed). Conversely,chronos
uses machine integer types likeInt
andInt64
, and only offers nanosecond resolution (10^-9 seconds)chronos
has put a lot more work into making its parsing functions fastA big question mark here is
postgresql-simple
—I'm not sure how much it in particular has slow parsing code, vs it being inherent toUTCTime
.My thoughts on this:
persistent-benchmark
package likepersistent-test
, that is then imported by the various backends?UTCTime
. Any ideas on fixes to this?UTCTime
faster? I suspect probably the code inpostgresql-simple
could be improved, but I have no basis for this (I've looked at the code and seen it in profiling results, but I'm not experienced with high performance Haskell enough to look at the code and immediately see why it's slow).chronos
be a good general purpose replacement forUTCTime
in Persistent? I would be hesitant about this since it's much less commonly used in the ecosystem.UTCTime
, would probably be faster than parsing directly to UTCTime