Conversation
Also cleaned up unused tickers
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Contributor
📝 WalkthroughWalkthroughIntroduces a new CachedClock implementation that caches time values at configurable intervals to minimize system calls, accompanied by comprehensive tests and benchmarks. Simultaneously removes ticker-related abstractions (NewTicker method and Ticker interface) from the Clock interface and all implementations (RealClock and TestClock). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CC as CachedClock
participant BG as Background Goroutine
participant Ticker
User->>CC: NewCachedClock(resolution)
activate CC
CC->>CC: Initialize with current time
CC->>BG: Start background goroutine
deactivate CC
par Periodic Updates
loop Every resolution interval
Ticker->>BG: Tick
BG->>CC: atomic.Store(nanos, time.Now().UnixNano())
end
and User Calls
loop Concurrent access
User->>CC: Now()
CC->>CC: atomic.Load(nanos)
CC-->>User: Return cached time
end
end
User->>CC: Close()
activate CC
CC->>BG: Signal close
CC->>Ticker: Stop()
deactivate CC
BG-->>BG: Exit goroutine
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
go/pkg/clock/cached_clock.go(1 hunks)go/pkg/clock/cached_clock_test.go(1 hunks)go/pkg/clock/clock_benchmarks_test.go(1 hunks)go/pkg/clock/interface.go(0 hunks)go/pkg/clock/real_clock.go(0 hunks)go/pkg/clock/test_clock.go(4 hunks)
💤 Files with no reviewable changes (2)
- go/pkg/clock/interface.go
- go/pkg/clock/real_clock.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-05T15:19:50.563Z
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3733
File: go/pkg/repeat/every_test.go:383-383
Timestamp: 2025-08-05T15:19:50.563Z
Learning: Go 1.24 introduced testing.B.Loop method which simplifies benchmark functions by handling iteration internally, ensuring setup/cleanup runs only once per benchmark count, and preventing compiler optimizations that could affect benchmark accuracy.
Applied to files:
go/pkg/clock/clock_benchmarks_test.go
🧬 Code graph analysis (3)
go/pkg/clock/cached_clock_test.go (2)
go/pkg/clock/cached_clock.go (2)
NewCachedClock(27-56)CachedClock(14-19)go/pkg/clock/interface.go (1)
Clock(12-18)
go/pkg/clock/cached_clock.go (1)
go/pkg/clock/interface.go (1)
Clock(12-18)
go/pkg/clock/clock_benchmarks_test.go (2)
go/pkg/clock/cached_clock.go (1)
NewCachedClock(27-56)go/pkg/clock/real_clock.go (1)
New(18-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: PR title
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
Flo4604
approved these changes
Nov 11, 2025
Member
Flo4604
left a comment
Flo4604
left a comment
There was a problem hiding this comment.
lgtm but rabbit has to be resolved
ogzhanolguncu
approved these changes
Nov 11, 2025
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (11/11/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
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
3 participants
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.
What does this PR do?
Adds a new
CachedClockimplementation that caches time values to reduce system call overhead. This implementation uses an atomic value that's updated at a configurable resolution by a background goroutine, providing better performance than directtime.Now()calls.The PR also simplifies the
Clockinterface by removing ticker-related functionality, focusing solely on time retrieval. Corresponding ticker implementations have been removed from bothRealClockandTestClock.Fixes # (issue)
Type of change
How should this be tested?
cd go
go test -bench=. ./pkg/clock/
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin main