-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Improve JetBrains port processing performance #20907
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 all commits
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 | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,6 +9,7 @@ import com.intellij.openapi.components.service | |||||||||
import com.intellij.openapi.diagnostic.thisLogger | ||||||||||
import com.intellij.remoteDev.util.onTerminationOrNow | ||||||||||
import com.intellij.ui.RowIcon | ||||||||||
import com.intellij.util.Alarm | ||||||||||
import com.intellij.util.application | ||||||||||
import com.jetbrains.rd.platform.codeWithMe.portForwarding.* | ||||||||||
import com.jetbrains.rd.util.URI | ||||||||||
|
@@ -35,6 +36,11 @@ abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService | |||||||||
private val ignoredPortsForNotificationService = service<GitpodIgnoredPortsForNotificationService>() | ||||||||||
private val lifetime = Lifetime.Eternal.createNested() | ||||||||||
|
||||||||||
// Throttling mechanism to prevent rapid successive port updates using IntelliJ Alarm | ||||||||||
private var pendingUpdate: Status.PortsStatusResponse? = null | ||||||||||
private val updateAlarm = Alarm(Alarm.ThreadToUse.SWING_THREAD, this) | ||||||||||
private val updateThrottleMs = 100 // 100ms throttle | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] The throttle interval is a magic number here. Consider extracting it into a named constant or configuration entry so it’s easier to adjust and document.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
|
||||||||||
init { start() } | ||||||||||
|
||||||||||
private fun start() { | ||||||||||
|
@@ -86,7 +92,7 @@ abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService | |||||||||
} | ||||||||||
|
||||||||||
override fun onNext(response: Status.PortsStatusResponse) { | ||||||||||
application.invokeLater { syncPortsListWithClient(response) } | ||||||||||
application.invokeLater { throttledSyncPortsListWithClient(response) } | ||||||||||
} | ||||||||||
|
||||||||||
override fun onCompleted() { | ||||||||||
|
@@ -103,6 +109,20 @@ abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService | |||||||||
return completableFuture | ||||||||||
} | ||||||||||
|
||||||||||
private fun throttledSyncPortsListWithClient(response: Status.PortsStatusResponse) { | ||||||||||
// Store the latest update (overwrites any pending update) | ||||||||||
pendingUpdate = response | ||||||||||
|
||||||||||
// Cancel any existing scheduled update and schedule a new one | ||||||||||
if (!updateAlarm.isEmpty) { | ||||||||||
updateAlarm.cancelAllRequests() | ||||||||||
} | ||||||||||
Comment on lines
+117
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] Checking
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||
updateAlarm.addRequest({ | ||||||||||
pendingUpdate?.let { syncPortsListWithClient(it) } | ||||||||||
pendingUpdate = null | ||||||||||
}, updateThrottleMs) | ||||||||||
} | ||||||||||
|
||||||||||
private fun isLocalPortForwardingDisabled(): Boolean { | ||||||||||
return System.getenv("GITPOD_DISABLE_JETBRAINS_LOCAL_PORT_FORWARDING")?.toBoolean() ?: false | ||||||||||
} | ||||||||||
|
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
this
as the parent Disposable may leak ifAbstractGitpodPortForwardingService
isn't disposable. Consider supplying a properDisposable
(e.g., the plugin’s lifecycle parent) or disposing the Alarm when the service shuts down.Copilot uses AI. Check for mistakes.