Frame interval calc possible optimization?

[tolgraven]

Have a bunch of old various minor changes and optimizations I finally took the time to rebase and try contribute back. Here is the first one.

[Copilot]

Pull request overview

This PR proposes optimizations to frame interval calculations and timing logic in the rendering loop. The changes include increasing the rolling window size for load tracking, improving warning messages, and modifying the frame scheduling algorithm.

• Increased the frame count for load tracking from 30 to 200 frames and made the ring buffer initialization consistent
• Modified the frame scheduling logic to use fixed intervals instead of adaptive timing
• Changed sleep behavior to skip sleeping entirely when frames run late

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The inline comment is excessively long and unclear. It appears to be exploratory thinking that should either be removed or moved to a proper block comment above the code. The comment also contains incomplete thoughts about alternative approaches without clearly stating which approach was chosen and why.

[Copilot]

This timing change introduces a subtle but potentially significant behavior modification. Previously, the next frame snapshot was calculated based on ended + sleep-time, which meant the next frame's timestamp was based on when the current frame ended plus the sleep time. Now it's calculated as ended + refresh-interval, which fixes the next frame to always be one full refresh interval after the current frame ended, regardless of how long the current frame took.

This means if a frame takes longer than the refresh interval (duration > refresh-interval), the next frame snapshot will be scheduled in the past relative to when it actually executes, since sleep-time will be 0 but the snapshot timestamp will only be refresh-interval milliseconds ahead.

Consider whether this is the intended behavior or if the snapshot should account for frames that run late. The comment suggests uncertainty about this design decision.

Suggested change

	next-frame-snapshot (rhythm/metro-snapshot (:metronome show) (+ ended (:refresh-interval show)))] ;or (+ ended (- (:refresh-interval show) duration)) to stay fully on clock... but unlikely to randomly manage to render next frame like, twice as fast as current. will have to judge what's feasible based on recent stats, active effects etc, and set a time
	next-frame-snapshot (rhythm/metro-snapshot (:metronome show) (+ ended sleep-time))] ; use actual scheduled delay so snapshot time doesn't end up in the past when frames overrun

[Copilot]

The conditional check when-not (zero? sleep-time) before calling Thread/sleep is unnecessary. Thread.sleep(0) is valid in Java and has well-defined behavior (it causes the thread to relinquish its current time slice to other threads). The original code's use of max 1 ensured at least 1ms sleep, which guaranteed context switching. With this change, when frames run late, the thread never yields, which could lead to CPU starvation of other threads or processes. Consider whether preventing the thread from ever yielding is the intended behavior.

Suggested change

	(when-not (zero? sleep-time) (Thread/sleep sleep-time))
	(Thread/sleep sleep-time)

[Copilot]

The :discarding field is added to the stats map but is never read or used anywhere in the codebase. This appears to be dead code. If this field was intended for debugging or monitoring purposes, consider removing it or documenting its purpose and adding code that actually uses it.

Suggested change

	(assoc stats :total-time total-time :frames-sent frames-sent :average-duration average-duration :discarding discarding
	:recent recent :recent-total recent-total :recent-average recent-average)))
	(assoc stats :total-time total-time
	:frames-sent frames-sent
	:average-duration average-duration
	:recent recent
	:recent-total recent-total
	:recent-average recent-average)))

[Copilot]

The inline comment is excessively long and difficult to read. It appears to be exploratory or draft thinking that should either be removed or moved to a proper block comment above the code. The comment also contains incomplete thoughts (e.g., "will have to judge what's feasible based on recent stats, active effects etc, and set a time") that make it unclear whether the implementation is complete or still needs work.

[brunchboy]

Thanks for submitting all these. It’s going to take me longer to look at most because I am currently digesting some significant contributions to my Pro DJ Link related projects, and I live much more in that world these days. I’ll also be visiting my sister’s family for Christmas, and have a DJ gig to prepare for right at the start of the new year. But I appreciate them and will give them thought as soon as I can!