eclipse-platform / eclipse.platform.text

8 stars 45 forks source link

Fix UI freeze when searching #189

Closed jjohnstn closed 1 year ago

jjohnstn commented 1 year ago
jjohnstn commented 1 year ago

@iloveeclipse This is the other solution

iloveeclipse commented 1 year ago

Jeff, I honestly have no idea what filters should filter in the patch (may be a comment in the code would help) and additionally I have no IDE access till weekend, so can't even debug or verify anything.

jjohnstn commented 1 year ago

@iloveeclipse I'll add a comment. The filter is set up to eliminate the same match being reported to two projects (one being nested in the other). The filter takes a FileMatch input and decides if it is being reported on the innermost project. The stream I am changing already is taking FileMatches and getting the line matches. In the loop, I was eventually seeing if the line match should be filtered because it's matched file is an outer duplicate. The count isn't relevant, it is whether the reported resource matches should be filtered. I have moved the check up so the line matches are already cleared. No count required and the filtering check is quick at this point. I could make it even quicker by checking for the filter length and doing one of two calculations of the line matches where one does filter checking and the other doesn't.

I have verified that my solution continues to filter out duplicates in the file search results. I'm sure yours will do the same.

Will either solution be able to be put into 4.28 M2? I can't merge platform changes.

iloveeclipse commented 1 year ago

I would favor yours, since you know what the code is doing, and I just had some minutes between meetings to look at it :) Please add comments to the code and I can merge.

jjohnstn commented 1 year ago

@iloveeclipse Let me know if the comment is ok. I added the check for active filters so the logic behaves exactly as it did before and should match or exceed the performance of #188.

iloveeclipse commented 1 year ago

Jeff, the whole elementsChanged() code wasn't easy to understand and filtering added on top didn't make it easier. Your comment before on this PR explained things better - could you please add something like this to the javadoc of elementsChanged()?

iloveeclipse commented 1 year ago

Thanks Jeff!