Skip to content
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

Make messageQueue.msgAndCtxs a circular buffer #2433

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

danlaine
Copy link

@danlaine danlaine commented Dec 6, 2023

Why this should be merged

We can remove some logic about slice manipulation. Also this should result in fewer slices being allocated.

How this works

[]*msgAndCtx --> buffer.Deque[*msgAndCtx]

How this was tested

Existing UT

@danlaine danlaine added the cleanup Code quality improvement label Dec 6, 2023
@danlaine danlaine added this to the v1.10.18 milestone Dec 6, 2023
@danlaine danlaine self-assigned this Dec 6, 2023
@@ -85,6 +86,7 @@ func NewMessageQueue(
cpuTracker: cpuTracker,
cond: sync.NewCond(&sync.Mutex{}),
nodeToUnprocessedMsgs: make(map[ids.NodeID]int),
msgAndCtxs: buffer.NewUnboundedDeque[*msgAndContext](1 /*=initSize*/),
Copy link
Contributor

@joshua-kim joshua-kim Dec 6, 2023

Choose a reason for hiding this comment

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

We do this in our codebase but I don't understand why... IDEs will usually display the name of the parameter and in this case it's really not needed since we only have one possible parameter

Copy link
Contributor

Choose a reason for hiding this comment

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

vscode does not put in the name of the parameter (which most of the team uses)

@StephenButtolph StephenButtolph added this pull request to the merge queue Dec 6, 2023
Merged via the queue into dev with commit 4705218 Dec 6, 2023
16 checks passed
@StephenButtolph StephenButtolph deleted the handler-use-circular-buffer branch December 6, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants