Repository navigation
yesod-core: compute max-expires per call instead of a 24h worker thread - #1923
Draft
parsonsmatt wants to merge 1 commit into
Draft
parsonsmatt wants to merge 1 commit into
parsonsmatt wants to merge 1 commit into
Conversation
getGetMaxExpires cached its value (a far-future RFC1123 timestamp) behind a mkAutoUpdate worker on a 24-hour refresh cycle. Because the refresh interval is a full day, the worker thread lingers for up to 24h after its last use before it can notice it is idle and exit. Constructing a YesodRunnerEnv per request (as test harnesses such as hspec-yesod do) therefore piles up dormant worker threads proportional to the request count. The value is a single formatTime of (now + 1 year), so caching it behind a thread is a premature optimization; runFakeHandler already computes it inline. Recompute it on each call and drop the worker entirely. No API changes: getGetMaxExpires keeps its 'IO (IO Text)' type and the yreGetMaxExpires field is unchanged, so this is a patch release (1.7.0.1). Both YesodRunnerEnv construction sites (mkYesodRunnerEnv, toWaiAppLogger) benefit, as does any direct caller. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getGetMaxExpirescached its value behind amkAutoUpdateworker on a 24-hour refresh cycle. Because the refresh interval is a full day, the worker thread lingers for up to 24h after its last use before it can notice it's idle and exit.Constructing a
YesodRunnerEnvper request — which test harnesses likehspec-yesoddo — therefore accumulates dormant worker threads proportional to the request count. (The session date cacher and logger time cache don't have this problem: theirmkAutoUpdateworkers self-terminate quickly because their refresh intervals are seconds, which is also whyclientSessionDateCacher's "closer" is already a documented no-op.)The cached value is just
formatTimeofnow + 1 year— a single cheap call. Caching it behind a thread is a premature optimization;runFakeHandleralready computes it inline. This PR recomputes it on each call and drops the worker entirely.Compatibility
No API changes — patch release (
1.7.0.0→1.7.0.1):getGetMaxExpires :: IO (IO Text)— signature unchanged.yreGetMaxExpiresfield — unchanged.YesodRunnerEnvconstruction sites (mkYesodRunnerEnv,toWaiAppLogger) and any direct caller benefit automatically.Control.AutoUpdateimport inDispatch.hs(the dependency stays —clientSessionDateCacherstill uses it).Behavior change: the max-expires timestamp now advances continuously instead of jumping once per day. It's a far-future (
+1yr) value used for cache/expiry headers, so this has no practical effect.Testing
cabal build yesod-corecompiles clean. This unblocks per-requestYesodRunnerEnvconstruction in test harnesses without leaking worker threads.🤖 Generated with Claude Code