snapframework/heist — comment on commit 407aced835ae72f8e61b1bf0bee636d0ac382af2
File: src/Text/Templating/Heist/Splices/Cache.hs line 37
By gregorycollins:

BTW, something I've been meaning to bring up for a while -- we should discuss whether the CacheTagState should be an IORef.

IORef would be faster for the common case of reads, because you wouldn't need to take a lock -- two threads trying to evaluate the page would then not have to block on each other. There is a downside here, which is that simply using IORef means that two threads will sometimes attempt to update the cache at the same time, which means that they will both evaluate the splice. There are a couple of different ways we could go here.

One would be to push the MVar down into the leaves, i.e.:

data CacheData = CacheData {
_expiryTime :: !CTime
, _template :: !Template
, _modLock :: !(MVar ())
}

newtype CacheTagState = CTS (IORef (HashMap Text (IORef CacheData)))

This has the advantage of never blocking for the common read case. The logic here is tricky though, after you've taken the modification lock you need to re-check the IORef contents to see whether another thread has beaten you to the update.

A couple of other data structure things here: CTime is preferable to UTCTime (it's a machine integer), unordered-containers would be much better than Data.Map, and a CacheData with unpacked fields would be better than a tuple.