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

Don't evaluate the same type twice when renaming a method #3561

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

fedejeanne
Copy link
Contributor

Copy link
Contributor

@jukzi jukzi left a comment

Choose a reason for hiding this comment

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

Theoretically the PR looks good, but unfortunatly ReferenceBinding may have unexpected hash/equals contract, see #3412. It might work to use a IdentityHashSet instead.

@fedejeanne
Copy link
Contributor Author

IdentityHashSet is a test class and moving it to a production plugin might be an overkill. I emulated the behavior in the internal Wrapper class in my 2nd commit (which should be squashed before merging).

WDYT @jukzi ?

@jukzi
Copy link
Contributor

jukzi commented Jan 16, 2025

IdentityHashSet is a test clas

use java.util.IdentityHashMap or org.eclipse.jdt.internal.compiler.lookup.ReferenceBindingSetWrapper

@fedejeanne
Copy link
Contributor Author

Thank you for the hint! I used java.util.IdentityHashMap

@jukzi
Copy link
Contributor

jukzi commented Jan 16, 2025

please find out if this code is executed by any test

@fedejeanne
Copy link
Contributor Author

I found these:

  • org.eclipse.jdt.core.tests.model.TypeHierarchyTests
  • org.eclipse.jdt.ui.tests.core.TypeHierarchyTest
  • org.eclipse.jdt.core.tests.model.TypeHierarchyNotificationTests

They run through the code, they run fast and they all pass.

There may be more (I stopped the search after these 3 findings).

@jukzi
Copy link
Contributor

jukzi commented Jan 17, 2025

ok, org.eclipse.jdt.core.tests.model\workspace\TypeHierarchy\src\p2\I1.java is extended by both I3 and X, and Y extends both X and I3, which forms a type hierarchy with redundant superinterfaces.

@jukzi jukzi merged commit e3d4707 into eclipse-jdt:master Jan 17, 2025
9 checks passed
@fedejeanne fedejeanne deleted the slow_method_rename branch January 17, 2025 08:25
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.

Poor performance when renaming method
2 participants