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.

Sign up for GitHub

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

Fix WebSocket threads CPU usage #88

Draft
wants to merge 1 commit into
base: 1-dev
Choose a base branch
from
Draft

Fix WebSocket threads CPU usage #88

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions src/main/kotlin/io/kuzzle/sdk/protocol/WebSocket.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,14 @@ import java.io.IOException
import java.net.ConnectException
import java.net.SocketException
import java.util.concurrent.CompletableFuture
import java.util.concurrent.ConcurrentLinkedQueue
import java.util.concurrent.atomic.AtomicBoolean
import kotlin.concurrent.thread

open class WebSocket : AbstractProtocol {
protected open var ws: DefaultClientWebSocketSession? = null
private val host: String
private val port: Int
private val isSsl: Boolean
private val queue: ConcurrentLinkedQueue<String> = ConcurrentLinkedQueue()
private val queue: ArrayDeque<String> = ArrayDeque()
override var state: ProtocolState = ProtocolState.CLOSE
private val autoReconnect: Boolean
private val reconnectionDelay: Long
Expand Down Expand Up @@ -114,13 +112,11 @@ open class WebSocket : AbstractProtocol {
state = ProtocolState.OPEN
trigger(NetworkStateChangeEvent(ProtocolState.OPEN))

thread(start = true) {
while (ws != null) {
val payload = queue.poll()
if (payload != null) {
GlobalScope.launch {
ws?.send(Frame.Text(payload))
}
Comment on lines -117 to -123
Copy link
Member

Choose a reason for hiding this comment

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

By removing the thread logic you make the SDK synchrone which is not the purpose. You should maybe handle and clean created thread when reconnecting instead of make it synchrone

while (queue.isNotEmpty()) {
val payload = queue.removeFirstOrNull()
if (payload != null) {
GlobalScope.launch {
ws?.send(Frame.Text(payload))
}
}
}
Expand Down Expand Up @@ -176,13 +172,6 @@ open class WebSocket : AbstractProtocol {

// On connection success
stopRetryingToConnect.set(false)

// This thread is here to let JAVA run until the socket is closed
// In Kotlin this is handled by the block function above but for some reason in JAVA it is
// non blocking.
thread(start = true) {
while (state != ProtocolState.CLOSE) {}
}
} catch (e: Exception) {
when (e) {
is ConnectException,
Expand Down Expand Up @@ -223,6 +212,12 @@ open class WebSocket : AbstractProtocol {
}

override fun send(payload: KuzzleMap) {
queue.add(JsonSerializer.serialize(payload))
if (state == ProtocolState.RECONNECTING) {
queue.add(JsonSerializer.serialize(payload))
} else {
GlobalScope.launch {
ws?.send(Frame.Text(JsonSerializer.serialize(payload)))
}
}
}
}
Loading