-
-
Notifications
You must be signed in to change notification settings - Fork 685
websocket: use FixedQueue instead of Set #3283
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
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
4dfe172 to
cc4f3de
Compare
mcollina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ronag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not fixed_queue from node core?
There is no particular reason for this. |
|
In this use case, I prefer the simple linkedlist; Initialization of the array of 2048 entries takes time :) |
I think that is amortized quite quickly... |
KhafraDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have benchmarks?
What about? |
|
If it was about sending data, it would be 1.2 times faster since we changed to zero-copy. |
|
Yeah, sending messages. Do you have the data to backup the 1.2x faster claim? |
|
okey, look this |
Rewrite with fixed_queue! |
KhafraDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is queue being lazily created?
KhafraDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests are failing
|
I don't have time right now, so I will rewrite it in another PR. |
This reverts commit 0e415b2.
8667b10 to
c05988c
Compare
468d2d0 to
a88052b
Compare
Co-authored-by: Khafra <[email protected]>
@KhafraDev