-
Notifications
You must be signed in to change notification settings - Fork 15
Remove NIOAsyncChannel #8
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
Changes from 1 commit
c5cb4e7
1eee688
ac4d7d2
1980571
70c0d48
ce5c011
0c5587c
432de34
cec0c27
055812b
c9bffed
4980daf
1e757a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| import Logging | ||||||
| import NIOConcurrencyHelpers | ||||||
| import NIOCore | ||||||
| import NIOPosix | ||||||
| import NIOSSL | ||||||
|
|
@@ -46,6 +47,7 @@ public struct ValkeyConnection: Sendable { | |||||
| let logger: Logger | ||||||
| let channel: Channel | ||||||
| let configuration: ValkeyClientConfiguration | ||||||
| let isClosed: NIOLockedValueBox<Bool> | ||||||
|
|
||||||
| /// Initialize Client | ||||||
| private init( | ||||||
|
|
@@ -56,6 +58,7 @@ public struct ValkeyConnection: Sendable { | |||||
| self.channel = channel | ||||||
| self.configuration = configuration | ||||||
| self.logger = logger | ||||||
| self.isClosed = .init(false) | ||||||
| } | ||||||
|
|
||||||
| public static func connect( | ||||||
|
|
@@ -70,6 +73,14 @@ public struct ValkeyConnection: Sendable { | |||||
| return connection | ||||||
| } | ||||||
|
|
||||||
| public func close() -> EventLoopFuture<Void> { | ||||||
| if self.isClosed.withLockedValue({ $0 }) { | ||||||
| return channel.eventLoop.makeSucceededFuture(()) | ||||||
|
||||||
| return channel.eventLoop.makeSucceededFuture(()) | |
| return channel.eventLoop.makeSucceededVoidFuture() |
Outdated
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 do we use @discardableResult here?
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.
This will need an @inlinable
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.
in an ideal case we keep the CommandEncoder in the channel handler and reuse the same one over and over there, so that we can reuse the internal buffer again and again, which should lead to zero allocations in the long term :)
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.
This only works if we can go for handler.write(command: command) here.
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.
you need a compAndExchange here.
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.
Good point missed that.
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.
You could even use an Atomic for this ;)
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.
Using an atomic now. I did look at that initially, but just realised why that wasn't working
ValkeyConnectionwas a struct. I changed it to be a final class which I think is more correct.