eclipse.platform icon indicating copy to clipboard operation
eclipse.platform copied to clipboard

Fix build block user action (#472)

Open haideryaqoob720 opened this issue 11 months ago • 11 comments

Problem

The Eclipse IDE has a bug where certain user actions (such as saving edits or interacting with version management) are blocked during the build process in the Eclipse IDE, even though they are not dependent on the ongoing compilation. The blocking behavior should be limited to actions that explicitly rely on the build process, such as Run and Debug.

Solution

To refine the selectivity of the blocking behavior, implementations have been made to the identifyThreadAction method within ThreadJob.java. The updated logic ensures that only jobs explicitly related to User actions (such as edit, save, rename or interacting with VCS and so on) are not blocked during the build process.

Changes Implemented

Implemented identifyThreadAction() to distinguish between build-dependent jobs and UI-related threads. Make sure that user actions aren’t blocked by the build. For more details, refer to the related bug report: Eclipse Bug Report #329657 GitHub Eclipse Discussion

Demonstration

A short demo video showcasing the changes in action is available here: demo

haideryaqoob720 avatar Feb 18 '25 14:02 haideryaqoob720

Would you please react to the comment here?

akurtakov avatar Mar 05 '25 16:03 akurtakov

Would you please react to the comment here?

I've been working on replacing string-based identifiers with job families

haideryaqoob720 avatar Mar 06 '25 11:03 haideryaqoob720

Hey @akurtakov @iloveeclipse, I’ve introduced isUI() methods in LockManager and LockListener to detect the UI thread and skip waiting when isUI() is true. This change prevents the UI from getting blocked by background build jobs, keeping interactions responsive for the user.

Issue: The user’s actions were getting blocked while a build job was running. In other words, the UI thread was waiting for the background build process to complete before allowing user interaction.

What Changed: A new method, isUI(), has been introduced in both the LockManager and LockListener classes. This method checks if the current thread is the UI thread. The joinRun method in ThreadJob has been updated to call isUI(). Now, if the thread is identified as the UI thread, it will not be forced to wait for the background build to finish.

How It Solves the Issue: By detecting whether the calling thread is the UI thread, we ensure that UI interactions are no longer blocked by a long-running build job. This allows the user to continue interacting with the UI while the background build proceeds independently

izubaire avatar Mar 18 '25 16:03 izubaire

This pull request changes some projects for the first time in this development cycle. Therefore the following files need a version increment:

runtime/bundles/org.eclipse.core.jobs/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From b8af36af3fd32836e99faab0d0133ca3e07632bb Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Tue, 18 Mar 2025 17:08:02 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream


diff --git a/runtime/bundles/org.eclipse.core.jobs/META-INF/MANIFEST.MF b/runtime/bundles/org.eclipse.core.jobs/META-INF/MANIFEST.MF
index d3660e7a86..4e6320b6b9 100644
--- a/runtime/bundles/org.eclipse.core.jobs/META-INF/MANIFEST.MF
+++ b/runtime/bundles/org.eclipse.core.jobs/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.core.jobs; singleton:=true
-Bundle-Version: 3.15.500.qualifier
+Bundle-Version: 3.15.600.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.core.internal.jobs;x-internal:=true,
-- 
2.48.1

Further information are available in Common Build Issues - Missing version increments.

eclipse-platform-bot avatar Mar 18 '25 17:03 eclipse-platform-bot

Test Results

 1 518 files   -  80   1 518 suites   - 80   1h 39m 15s ⏱️ + 8m 45s  4 020 tests  - 153   3 994 ✅  - 156   23 💤 ±0  3 ❌ +3  11 542 runs   - 456  11 369 ✅  - 463  166 💤 ±0  7 ❌ +7 

For more details on these failures, see this check.

Results for commit 74ed88cc. ± Comparison against base commit 9e339b2f.

This pull request removes 153 tests.
AllCompareTests AsyncExecTests ‑ testCancelOnRequeue
AllCompareTests AsyncExecTests ‑ testQueueAdd
AllCompareTests AsyncExecTests ‑ testWorker
AllCompareTests CompareFileRevisionEditorInputTest ‑ testPrepareCompareInputWithNonLocalResourceTypedElements
AllCompareTests CompareUIPluginTest ‑ testFindContentViewerDescriptorForTextType_StreamAccessor
AllCompareTests CompareUIPluginTest ‑ testFindContentViewerDescriptor_TextType_NotStreamAccessor
AllCompareTests CompareUIPluginTest ‑ testFindContentViewerDescriptor_UnknownType
AllCompareTests ContentMergeViewerTest ‑ testFFFX
AllCompareTests ContentMergeViewerTest ‑ testFFTX
AllCompareTests ContentMergeViewerTest ‑ testFFXF
…

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Mar 26 '25 21:03 github-actions[bot]

Hey @iloveeclipse, thanks for the great feedback - pushed the suggested changes a while back. Big thanks to @akurtakov for the follow-up and to @izubaire for collaborating as part of the team. Would really appreciate it if you could take a look and share your feedback.

haideryaqoob720 avatar Apr 10 '25 13:04 haideryaqoob720

@haideryaqoob720 :

  1. there are two test failures caused by your patch: https://ci.eclipse.org/platform/job/eclipse.platform/job/PR-1743/6/testReport/
  2. A new test is needed to demonstrate the problem and the fix.

iloveeclipse avatar Apr 10 '25 14:04 iloveeclipse

The blocking behavior should be limited to actions that explicitly rely on the build process, such as Run and Debug.

I don't think this assertion is true. The strategy to decide whether to block an action or not is to rely on SchedulingRules. If a build job has a rule holding a file, then it's expected that the file being hold cannot be modified. In general, instead of trying to bend the Jobs framework, a saner and better strategy involves finding the Job that holds the rule causing the blocked operation and then trying to minimize the impact of its scheduling rule (ie not block a whole project, but only block the few files in needs to read, only for the shortest possible time).

mickaelistria avatar Apr 10 '25 14:04 mickaelistria

@iloveeclipse @akurtakov @Michael5601 Thanks for the valuable feedbacks. I’ve reviewed the SchedulingRules approach and agree it fits well with Eclipse’s design. However, since the current implementation of Build All job locks the entire workspace, adopting SchedulingRules would require significant refactoring to target specific resources - which is a broader architectural effort and and might require more coordination across the platform. Reference


For now, the isUI() API offers a practical, non-invasive fix to improve UI responsiveness without affecting scheduling safety.

Would appreciate team input on:

  • Is there a non-invasive way to use SchedulingRules under the current setup?
  • Or if it’s reasonable to proceed with the current isUI() solution

haideryaqoob720 avatar Apr 14 '25 17:04 haideryaqoob720

The point is that whether an operation is UI or not doesn't make a difference: if the operation uses a scheduling rule that is lock, then it must wait for the scheduling rule to be free; otherwise a lot of bugs can happen. Please see https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Fguide%2Fruntime_jobs_rules.htm or https://www.eclipse.org/articles/Article-Concurrency/jobs-api.html ; anything that tries to just ignore that is basically introducing big risks of bugs. The "Build All" operation could be refactored if some reliable optimizations can be found. Usually, not using rules or using more "narrow" rules helps a lot. I don't see a particular reason why "Build All" would require to lock the whole workspace for the duration of the build; as it mostly reads the projects to establish a build order and then each individual builder on the project sets a scheduling rule to lock the right resources. So it could be that just not setting a rule on Build All would be a good solution.

mickaelistria avatar Apr 14 '25 19:04 mickaelistria

@iloveeclipse @akurtakov @mickaelistria To enhance UI responsiveness during builds while preserving SchedulingRules integrity:

  • Fine-Grained Locking:
    • Build All: Lock and build projects individually instead of the entire workspace lock.
    • Build Project: Lock only the specific project or finer where possible.
    • Auto-Build: Maintain minimal locking (file/resource level) wherever feasible.
  • Resource Tracking: Detect file edits during builds and trigger incremental rebuilds.
  • SchedulingRules Compliance: Fully honor concurrency guarantees without bypassing locks. This approach improves responsiveness safely and aligns Eclipse behavior closer to modern IDEs. I’d appreciate your feedbacks. Thanks!

haideryaqoob720 avatar Apr 28 '25 22:04 haideryaqoob720