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

CircularBuffer: use vendored swift-collections/Deque instead #1792

Closed
wants to merge 1 commit into from

Conversation

weissi
Copy link
Member

@weissi weissi commented Apr 6, 2021

Motivation:

Less (SwiftNIO maintained) code is better and Swift Collection's got an
excellent Deque implementation that NIO should just use.

Modification:

Use Swift Collection's Deque to back CircularBuffer instead rolling our
own (which is certainly much slower).

Result:

  • Less (NIO maintained) code
  • faster

Motivation:

Less (SwiftNIO maintained) code is better and Swift Collection's got an
excellent Deque implementation that NIO should just use.

Modification:

Use Swift Collection's Deque to back CircularBuffer instead rolling our
own (which is certainly much slower).

Result:

- Less (NIO maintained) code
- faster
@weissi
Copy link
Member Author

weissi commented Apr 6, 2021

@swift-nio-bot test perf please

@weissi
Copy link
Member Author

weissi commented Apr 6, 2021

CC @Lukasa / @lorentey

@@ -0,0 +1,23 @@
/* Changes for SwiftNIO
- renamed NIODeque to NIONIODeque (to prevent future clashes)
- made (NIO)NIODeque internal (not public)
Copy link
Contributor

Choose a reason for hiding this comment

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

Something here is prefixing NIO again.

@weissi weissi closed this Jun 1, 2021
@ktoso
Copy link
Member

ktoso commented Jun 2, 2021

Oh, awesome :-)

@weissi
Copy link
Member Author

weissi commented Jun 2, 2021

Oh, awesome :-)

Except that it doesn’t work 😂😂. https://forums.swift.org/t/rfc-deque-should-it-have-an-opaque-index/47203

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.

None yet

3 participants