Search View: Performance issues on remove and sort by mehmet-karaman · Pull Request #3733 · eclipse-platform/eclipse.platform.ui

mehmet-karaman · 2026-02-17T14:17:58Z

Implemented a batch removal of search matches
Search matches are removed from their parent, instead of one by one
which causes performance issues
It's also fixing the problem, that all elements are removed instead
only the visible elements (while all others are filtered by the limit
number)
Replaces the
ConcurrentHashMap.newKeySet() with the ConcurrentSkipListSet with the
comparator and makes the additional sorting obsolete.

iloveeclipse · 2026-02-17T14:21:47Z

@mehmet-karaman : please first create a ticket and explain the problem.
Note: you seem to use wrong git author mail, please use same which you've used in ECA process.

github-actions · 2026-02-17T15:34:19Z

Test Results

3 018 files - 6 3 018 suites - 6 2h 18m 38s ⏱️ + 1m 54s
8 237 tests + 3 7 989 ✅ + 3 248 💤 ±0 0 ❌ ±0
23 509 runs - 17 22 721 ✅ - 14 788 💤 - 3 0 ❌ ±0

Results for commit 27539cf. ± Comparison against base commit 89a19bf.

♻️ This comment has been updated with latest results.

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

Copilot

Pull request overview

This PR addresses Search View performance and correctness when removing matches, by enabling batch removal at the “enclosing element” (e.g., file/resource) level and ensuring removals aren’t limited to only currently visible (element-limited) results.

Changes:

Updated AbstractTextSearchViewPage.internalRemoveSelected() to batch-remove matches by selected IFile / IContainer resources before falling back to per-match removal for non-resource selections.
Added AbstractTextSearchResult.removeElements(...) to remove all matches for selected elements via a single map-entry removal per element.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java`	Adds resource-aware pre-processing in “Remove Selected Matches” to remove matches by file/container in a batch.
`bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java`	Introduces an element-based batch removal method to efficiently drop all matches for selected elements.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java

iloveeclipse · 2026-02-23T11:44:30Z

@iloveeclipse I've squashed your two commits on my last commit..

There is compile error now

mehmet-karaman · 2026-02-23T12:11:26Z

Forgot one file in stage.. Now its done..

iloveeclipse · 2026-02-23T13:50:06Z

@mehmet-karaman : please check failing tests: https://github.com/eclipse-platform/eclipse.platform.ui/runs/64528670814

mehmet-karaman · 2026-02-23T15:42:17Z

Seems that the default is table view and not tree.. I've added a layout call.. Locally it worked.

mehmet-karaman · 2026-02-23T16:11:16Z

I am going to try to reproduce the failure locally..

iloveeclipse · 2026-02-23T16:21:07Z

Seems that the default is table view and not tree.

I assume this be forced by using right preference for the search, but I haven't checked what is the probllem with the test. Note: you must "process UI events loop" in order to see fully updated search view after some search job is executed, this could be the problem you observe.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T20:15:21Z

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

+		int diff = match2.getOffset() - match1.getOffset();
+		if (diff != 0) {
+			return diff;
+		}
+		diff = match2.getLength() - match1.getLength();
+		if (diff != 0) {
+			return diff;
+		}
+		diff = System.identityHashCode(match2) - System.identityHashCode(match1);


The comparator for ConcurrentSkipListSet uses integer subtraction (offset/length/identityHashCode). These subtractions can overflow and violate the comparator contract, which can break ordering/lookup behavior in a skip-list. Prefer Integer.compare(...) for offset/length/hash comparisons to guarantee correct ordering.

Suggested change

int diff = match2.getOffset() - match1.getOffset();

if (diff != 0) {

return diff;

}

diff = match2.getLength() - match1.getLength();

if (diff != 0) {

return diff;

}

diff = System.identityHashCode(match2) - System.identityHashCode(match1);

int diff = Integer.compare(match2.getOffset(), match1.getOffset());

if (diff != 0) {

return diff;

}

diff = Integer.compare(match2.getLength(), match1.getLength());

if (diff != 0) {

return diff;

}

diff = Integer.compare(System.identityHashCode(match2), System.identityHashCode(match1));

Copilot · 2026-02-23T20:15:22Z

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

+			}
+		}
+		if (!removedMatches.isEmpty()) {
+			removedMatches.forEach(collisionOrder::remove);


removeElements(...) calls collisionOrder.remove(...) for every removed match, even though collisionOrder will almost always be empty (identityHash collisions are very rare). For large batch removals this adds unnecessary per-match overhead. Consider guarding the loop with a single if (!collisionOrder.isEmpty()) check (or otherwise avoiding per-match removals when no collisions were recorded).

Suggested change

removedMatches.forEach(collisionOrder::remove);

if (!collisionOrder.isEmpty()) {

removedMatches.forEach(collisionOrder::remove);

}

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

mehmet-karaman · 2026-02-23T22:15:57Z

Seems that the default is table view and not tree.

I assume this be forced by using right preference for the search, but I haven't checked what is the probllem with the test. Note: you must "process UI events loop" in order to see fully updated search view after some search job is executed, this could be the problem you observe.

Your guess was correct, It was failing due to pending UI events.. The tests are running now, but another test is failing. Could be that I have to set the default value back in tearDown.. This is also pushed now.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

Copilot · 2026-02-25T07:31:42Z

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

+		resViewer.setSelection(new StructuredSelection(children[0]));
+
+		currentPage.internalRemoveSelected();
+
+		assertEquals(0, result.getMatchCount(), "Correct number of matches removed");


The test assumes that all matches in the workspace belong to a single folder (the first child of the first project), expecting getMatchCount() to be 0 after removal. This assumption may not hold if there are multiple folders with matches. The test should either verify the assumption or compute the expected remaining match count more precisely.

Suggested change

resViewer.setSelection(new StructuredSelection(children[0]));

currentPage.internalRemoveSelected();

assertEquals(0, result.getMatchCount(), "Correct number of matches removed");

int folderMatchCount= result.getMatchCount(children[0]);

resViewer.setSelection(new StructuredSelection(children[0]));

currentPage.internalRemoveSelected();

int expectedCount= originalCount - folderMatchCount;

assertEquals(expectedCount, result.getMatchCount(), "Correct number of matches removed");

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

Copilot · 2026-02-25T09:49:20Z

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

+		assertTrue(directElements.length == 1, "Should have only one direct element");
+		assertTrue(directElements[0] instanceof IProject, "Should be a project");
+		resViewer.setSelection(new StructuredSelection(directElements[0]));


The tests assume the workspace search produces exactly one top-level element (a single IProject) and that the project has exactly one child folder. This is brittle if other tests/projects exist in the workspace or if the JUnitSource project structure changes; consider scoping the FileTextSearchScope to the JUnitSource project (or locating the project/folder by name/path) instead of asserting array lengths are 1.

Suggested change

assertTrue(directElements.length == 1, "Should have only one direct element");

assertTrue(directElements[0] instanceof IProject, "Should be a project");

resViewer.setSelection(new StructuredSelection(directElements[0]));

Object projectElement= null;

for (Object element : directElements) {

if (element instanceof IProject) {

projectElement= element;

break;

}

}

assertTrue(projectElement != null, "Should contain a project element");

resViewer.setSelection(new StructuredSelection(projectElement));

Copilot · 2026-02-25T09:49:20Z

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

+		int folderMatchCount= result.getMatchCount();
+		resViewer.setSelection(new StructuredSelection(children[0]));
+
+		currentPage.internalRemoveSelected();
+
+		int expectedCount= originalCount - folderMatchCount;
+		assertEquals(expectedCount, result.getMatchCount(), "Correct number of matches removed");


In testBatchRemoveElementsByFolder, folderMatchCount is taken from result.getMatchCount(), which is the total match count, making expectedCount always 0 and largely duplicating the project-removal assertion. To actually validate folder-specific removal, compute the number of matches under the selected folder (e.g., by summing match counts of result elements whose paths are under that folder) and assert only those were removed.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T10:25:27Z

tests/org.eclipse.search.tests/src/org/eclipse/search/tests/TextSearchResultTest.java

+		String[] fileNamePatterns= { "*.java" };
+		FileTextSearchScope scope= FileTextSearchScope.newWorkspaceScope(fileNamePatterns, false);
+
+		fQuery1= new FileSearchQuery("Test", false, true, scope);
+		NewSearchUI.runQueryInForeground(null, fQuery1);
+


In setUp(), the query runs on a workspace-wide scope. The later tests assume a single top-level project/folder in the tree, which can become flaky if other tests leave additional projects in the workspace (or if tests run in parallel). Consider scoping the search to the JUnitSource project created by JUnitSourceSetup (e.g., via FileTextSearchScope.newSearchScope with that project) so the viewer structure is deterministic.

- Implemented a batch removal of search matches - Search matches are removed from their parent, instead of one by one which causes performance issues - It's also fixing the problem, that all elements are removed instead only the visible elements (while all others are filtered by the limit number) - Replaces the ConcurrentHashMap.newKeySet() with the ConcurrentSkipListSet with the comparator and makes the additional sorting obsolete. - After this change, avoid using `size()` on values, because it is not more constant time operation, see `ConcurrentSkipListSet` javadoc. - Also comparator must be changed to consider different Match elements with same size and offsets in the set - otherwise the `ConcurrentSkipListSet` would lost them. - Added `AbstractTextSearchResult.hasMatches(Object element)` API for cases where match count not needed. - If possible, calls to `size()` changed to `isEmpty()` or omitted. - Implemented Tests Fixes eclipse-platform#3735 Co-authored-by: Andrey Loskutov <loskutov@gmx.de>

mehmet-karaman force-pushed the batch_remove_search_matches branch from 2a62c61 to 73cab0d Compare February 17, 2026 14:52

mehmet-karaman force-pushed the batch_remove_search_matches branch 4 times, most recently from dff7693 to 300ca6b Compare February 17, 2026 23:07

iloveeclipse requested changes Feb 18, 2026

View reviewed changes

mehmet-karaman force-pushed the batch_remove_search_matches branch from 300ca6b to fe5e861 Compare February 18, 2026 22:21

mehmet-karaman commented Feb 18, 2026

View reviewed changes

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java Outdated Show resolved Hide resolved

mehmet-karaman force-pushed the batch_remove_search_matches branch from fe5e861 to 7f02394 Compare February 18, 2026 23:19

iloveeclipse requested a review from Copilot February 19, 2026 08:03

Copilot started reviewing on behalf of iloveeclipse February 19, 2026 08:04 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

fedejeanne mentioned this pull request Feb 19, 2026

Search view with millions of matches is freezing the UI #3735

Open

mehmet-karaman force-pushed the batch_remove_search_matches branch 2 times, most recently from 4cd364f to b53503b Compare February 19, 2026 11:20

mehmet-karaman changed the title ~~Search View: Batch removing of search matches~~ Search View: Performance issues on remove and sort Feb 19, 2026

iloveeclipse requested a review from Copilot February 19, 2026 11:56

Copilot started reviewing on behalf of iloveeclipse February 19, 2026 11:56 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java Outdated Show resolved Hide resolved

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java Outdated Show resolved Hide resolved

mehmet-karaman force-pushed the batch_remove_search_matches branch 2 times, most recently from 17ec4af to fc2f401 Compare February 19, 2026 12:46

iloveeclipse reviewed Feb 19, 2026

View reviewed changes

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java Outdated Show resolved Hide resolved

iloveeclipse force-pushed the batch_remove_search_matches branch from fc2f401 to 40ee202 Compare February 19, 2026 13:48

iloveeclipse requested a review from Copilot February 19, 2026 14:12

Copilot started reviewing on behalf of iloveeclipse February 19, 2026 14:12 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

iloveeclipse force-pushed the batch_remove_search_matches branch from 40ee202 to 7d37158 Compare February 19, 2026 15:13

mehmet-karaman force-pushed the batch_remove_search_matches branch from 7d37158 to d572e09 Compare February 20, 2026 11:22

mehmet-karaman force-pushed the batch_remove_search_matches branch from d081dad to 3dc231e Compare February 23, 2026 12:12

mehmet-karaman force-pushed the batch_remove_search_matches branch from 3dc231e to 98df1af Compare February 23, 2026 15:40

iloveeclipse requested a review from Copilot February 23, 2026 20:09

Copilot started reviewing on behalf of iloveeclipse February 23, 2026 20:10 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

mehmet-karaman force-pushed the batch_remove_search_matches branch 2 times, most recently from d53e827 to 47c1437 Compare February 23, 2026 22:08

mehmet-karaman force-pushed the batch_remove_search_matches branch 2 times, most recently from 26d440a to c7599f8 Compare February 24, 2026 11:10

iloveeclipse requested a review from Copilot February 25, 2026 07:26

Copilot started reviewing on behalf of iloveeclipse February 25, 2026 07:27 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

mehmet-karaman force-pushed the batch_remove_search_matches branch 2 times, most recently from 07fe961 to 127e48e Compare February 25, 2026 09:31

iloveeclipse requested a review from Copilot February 25, 2026 09:41

Copilot started reviewing on behalf of iloveeclipse February 25, 2026 09:41 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

mehmet-karaman force-pushed the batch_remove_search_matches branch from 127e48e to 66dda3f Compare February 25, 2026 10:10

iloveeclipse requested a review from Copilot February 25, 2026 10:19

Copilot started reviewing on behalf of iloveeclipse February 25, 2026 10:19 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

mehmet-karaman force-pushed the batch_remove_search_matches branch from 66dda3f to 27539cf Compare February 25, 2026 13:12

-		assertTrue(directElements.length == 1, "Should have only one direct element");
-		assertTrue(directElements[0] instanceof IProject, "Should be a project");
-		resViewer.setSelection(new StructuredSelection(directElements[0]));
+		Object projectElement= null;
+		for (Object element : directElements) {
+			if (element instanceof IProject) {
+				projectElement= element;
+				break;
+			}
+		}
+		assertTrue(projectElement != null, "Should contain a project element");
+		resViewer.setSelection(new StructuredSelection(projectElement));

Conversation

mehmet-karaman commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iloveeclipse commented Feb 17, 2026

Uh oh!

github-actions bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iloveeclipse commented Feb 23, 2026

Uh oh!

mehmet-karaman commented Feb 23, 2026

Uh oh!

iloveeclipse commented Feb 23, 2026

Uh oh!

mehmet-karaman commented Feb 23, 2026

Uh oh!

mehmet-karaman commented Feb 23, 2026

Uh oh!

iloveeclipse commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mehmet-karaman commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

mehmet-karaman commented Feb 17, 2026 •

edited

Loading

github-actions bot commented Feb 17, 2026 •

edited

Loading