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.

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

Avoid unnecessary lock on UI thread with no model listeners #3252

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mx990
Copy link
Contributor

@mx990 mx990 commented Nov 22, 2024

Avoid unnecessarily aquiring a lock on the UI thread if there are no model listeners in the Xtext document.

Closes #3251

Copy link

github-actions bot commented Nov 22, 2024

Test Results

  6 460 files  ±0    6 460 suites  ±0   3h 18m 10s ⏱️ + 6m 8s
 43 238 tests ±0   42 654 ✅ ±0    584 💤 ±0  0 ❌ ±0 
170 249 runs   - 2  167 904 ✅ ±0  2 336 💤 ±0  9 ❌  - 1 

Results for commit da7fed8. ± Comparison against base commit 4eebe72.

♻️ This comment has been updated with latest results.

@@ -563,6 +563,9 @@ protected <T> T internalReadOnly(IUnitOfWork<T, XtextResource> work, boolean isC
* changing while they run. To achieve this, we run the {@link IXtextModelListener}s on the UI thread.
*/
private void notifyModelListenersOnUiThread() {
if(modelListeners.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The modelListeners should not be accessed outside of a synchronization block. Please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@mx990 mx990 force-pushed the documentLockerNotifyModelListeners branch from 7783fd5 to 8a80d14 Compare November 25, 2024 13:51
@cdietrich
Copy link
Contributor

can you please rebase

…text#3251

Avoid unnecessarily aquiring a lock on the UI thread if there are no
model listeners in the Xtext document.

Signed-off-by: Martin Jobst <[email protected]>
@mx990 mx990 force-pushed the documentLockerNotifyModelListeners branch from 8a80d14 to da7fed8 Compare November 27, 2024 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid unnecessary lock on UI thread with no model listeners
3 participants