Skip to content

Change VecDeque for fixed-capacity ArrayDeque in LinearRegression #2667

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

Merged
merged 1 commit into from
May 26, 2025

Conversation

nicolad
Copy link
Collaborator

@nicolad nicolad commented May 25, 2025

Why touch the buffer in LinearRegression at all?

LinearRegression (LR) keeps a sliding window whose length (period) is known a-priori.
With the current std::collections::VecDeque the queue still grows past that length: every push_back that exceeds capacity triggers a heap re-allocation and mem-copy. Those reallocations are

  • wasted work — we immediately pop_front afterwards, and
  • a latency spike right on the trading hot-path.

arraydeque::ArrayDeque, in contrast, is “a circular buffer with fixed capacity … [that] can be stored directly on the stack”¹.
Its capacity is a const generic baked into the type, so pushes never allocate and the window length is never exceeded.

Aspect VecDeque ArrayDeque<N>
Capacity Dynamic; grows geometrically Compile-time fixed (const N)
Allocation Heap (may reallocate) None (stack/static)
Push/Pop cost Amortised O(1) but occasional mem-copy Strict O(1)
Enforced upper bound Caller-dependent, not guaranteed Compile-time assert; panic if period > N
Max window length usize::MAX (practically unbounded) Chosen at build time (MAX_PERIOD = 16 384)
Failure mode when full Reallocate + copy Programmer picks Wrapping (overwrite) or Saturating (reject)

@nicolad nicolad requested a review from cjdsellers May 25, 2025 12:47
@nicolad nicolad self-assigned this May 25, 2025
Copy link
Member

@cjdsellers cjdsellers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 thanks @nicolad this is a good optimization.

@nicolad nicolad force-pushed the 2507-arraydeque-2 branch from 9997c23 to a3e16a1 Compare May 26, 2025 06:59
@nicolad nicolad force-pushed the 2507-arraydeque-2 branch from a3e16a1 to ff960cd Compare May 26, 2025 07:15
@nicolad
Copy link
Collaborator Author

nicolad commented May 26, 2025

image

@cjdsellers cjdsellers merged commit 7cc4600 into develop May 26, 2025
17 checks passed
@cjdsellers cjdsellers deleted the 2507-arraydeque-2 branch May 26, 2025 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants