Skip to content

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

Merged
merged 2 commits into from
Jun 17, 2025

Conversation

mustard-mh
Copy link
Contributor

@mustard-mh mustard-mh commented Jun 17, 2025

Add throttling mechanism to prevent rapid successive port updates, reducing UI flicker and improving performance in the JetBrains backend plugin.

🤖 Generated with Claude Code

Description

Related Issue(s)

Relates CLC-1260

How to test

Integration tests
https://github.com/gitpod-io/gitpod/actions/runs/15700198958/job/44233991339?pr=20907

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

Build Options

Build
  • /werft with-werft
    Run the build with werft instead of GHA
  • leeway-no-cache
  • /werft no-test
    Run Leeway with --dont-test
Publish
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer
  • analytics=segment
  • with-dedicated-emulation
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • /werft preemptible
    Saves cost. Untick this only if you're really sure you need a non-preemtible machine.
  • with-integration-tests=jetbrains
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
  • with-monitoring

/hold

Add throttling mechanism to prevent rapid successive port updates, reducing UI flicker and improving performance in the JetBrains backend plugin.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a throttling mechanism to batch rapid port updates in the JetBrains plugin, reducing UI flicker and improving performance.

  • Adds an IntelliJ Alarm to delay and merge successive PortsStatusResponse updates
  • Replaces direct calls to syncPortsListWithClient with a new throttledSyncPortsListWithClient wrapper
  • Introduces pendingUpdate storage and a 100 ms throttle interval
Comments suppressed due to low confidence (1)

components/ide/jetbrains/backend-plugin/src/main/kotlin/io/gitpod/jetbrains/remote/AbstractGitpodPortForwardingService.kt:112

  • There aren’t unit tests for the new throttling logic. Adding tests to simulate rapid successive responses and verifying that only the latest update is processed would ensure correctness.
private fun throttledSyncPortsListWithClient(response: Status.PortsStatusResponse) {

@@ -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)
Copy link
Preview

Copilot AI Jun 17, 2025

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 if AbstractGitpodPortForwardingService isn't disposable. Consider supplying a proper Disposable (e.g., the plugin’s lifecycle parent) or disposing the Alarm when the service shuts down.

Suggested change
private val updateAlarm = Alarm(Alarm.ThreadToUse.SWING_THREAD, this)
private val updateAlarm = Alarm(Alarm.ThreadToUse.SWING_THREAD, application)

Copilot uses AI. Check for mistakes.

// 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
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

The 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
private val updateThrottleMs = 100 // 100ms throttle
private val updateThrottleMs = DEFAULT_UPDATE_THROTTLE_MS // Default throttle interval

Copilot uses AI. Check for mistakes.

Comment on lines +117 to +119
if (!updateAlarm.isEmpty) {
updateAlarm.cancelAllRequests()
}
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

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

[nitpick] Checking isEmpty before cancelAllRequests() may be unnecessary, since cancelAllRequests() is idempotent. You could simplify by always calling cancelAllRequests() or guard by isDisposed if needed.

Suggested change
if (!updateAlarm.isEmpty) {
updateAlarm.cancelAllRequests()
}
updateAlarm.cancelAllRequests()

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@iQQBot iQQBot left a comment

Choose a reason for hiding this comment

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

image

@roboquat roboquat merged commit a0a2dc6 into main Jun 17, 2025
92 of 93 checks passed
@roboquat roboquat deleted the throttle-jetbrains-port-updates branch June 17, 2025 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants