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

[Performance] Full build takes more time since 2024-09 (type inference) #3384

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

stephan-herrmann
Copy link
Contributor

optimize 1:

  • don't record pairs without any type arguments

optimize 2:

  • stop traversal when both types are the same

Fixes #3327

optimize 1:
+ don't record pairs without any type arguments

optimize 2:
+ stop traversal when both types are the same

Fixes eclipse-jdt#3327
@stephan-herrmann
Copy link
Contributor Author

First observation: allSuperPairsWithCommonGenericType() has two clients, both of which are only interested in pairs of types that contain some type arguments:

  • condition18_5_2_bullet_3_3_1(): Here the method is used to check this rule:
    • S1 and S2 have supertypes (4.10) that are two different parameterizations of the same generic class or interface.
      If no type is parameterized then this condition can never be satisfied
  • deriveTypeArgumentConstraints(): Here pairs are piped into typeArgumentEqualityConstraints() where emptyList() is returned if either type has no type arguments.

Ergo it's safe to skip pairs without any type arguments. Perhaps my checks could even be strengthened: currently we proceed if only one type is parameterized.

Second observation: We continue traversal to super types even when s and t are essentiall the same type. If starting from here we would ever encounter common supertypes with different parameterization, that hierarchy would be illegal to begin with:
The interface A cannot be implemented more than once with different arguments: A<Integer> and A<String>

Erog it's safe to stop traveral when TypeBinding.equalsEquals(s,t) (using equalsEquals rather than == because different type annotations can safely be ignored).

@stephan-herrmann
Copy link
Contributor Author

@fedejeanne the first optimization is effective if many non-generic types are involved. Does your original code contain lots of generics? If so, this particular optimization may not help a lot.

@stephan-herrmann
Copy link
Contributor Author

I repeated the measurements from #3333 (comment) after applying both optimizations:

ecj

min max
real 0m4,210s 0m6,360s
user 0m8,628s 0m9,131s
sys 0m0,316s 0m0,393s

for reference the original results

ecj

min max
real 0m8,335s 0m8,799s
user 0m13,848s 0m14,265s
sys 0m0,427s 0m0,489s

javac

min max
real 0m14,618s 0m16,737s
user 0m19,790s 0m20,369s
sys 0m0,293s 0m0,436s

I'm curious how this micro benchmark will translate into practical experience ...

strengthen optimization 1:
+ record only pairs where both types are parameterized

Fixes eclipse-jdt#3327
@jukzi
Copy link
Contributor

jukzi commented Dec 3, 2024

It's an improvement, but does not fix the O(n^2) behavior.
For example try #3327 (comment) with I40. You better save your workspace before-

@stephan-herrmann stephan-herrmann merged commit 400bddd into eclipse-jdt:master Dec 3, 2024
10 checks passed
@stephan-herrmann stephan-herrmann deleted the issue3327 branch December 3, 2024 17:33
stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this pull request Dec 3, 2024
(eclipse-jdt#3384)

optimize 3:
+ never visit the same super type more than once

Fixes eclipse-jdt#3327
@stephan-herrmann
Copy link
Contributor Author

It's an improvement, but does not fix the O(n^2) behavior. For example try #3327 (comment) with I40. You better save your workspace before-

See #3387

@stephan-herrmann stephan-herrmann added this to the 4.35 M1 milestone Dec 3, 2024
@fedejeanne
Copy link
Contributor

@fedejeanne the first optimization is effective if many non-generic types are involved. Does your original code contain lots of generics? If so, this particular optimization may not help a lot.

As a rule of thumb, our code contains a lot of everything (it's just a huge code base 😅 ). I will try this optimization and see how much performance was gained.

@fedejeanne
Copy link
Contributor

@stephan-herrmann I just compiled the whole workspace with yesterday's I-BUILD (v20241203-1917) and it went down from ~30 min to ~17 min! 👏

Here's a sample (sampled every 20ms):
v20241203-1907-Full build (clean workspace).zip

And here one can see that even though allSuperPairsWithCommonGenericType is called as often as before, it is faster and that findSuperTypesOriginatingFrom is called more often (???) but it takes less time. I would take these assumptions with a pinch of salt though, since I am comparing with the original screenshots in #3327 (comment) but I don't remember if back then I sampled every 100ms or every 20ms.

image

@jukzi
Copy link
Contributor

jukzi commented Dec 4, 2024

@fedejeanne The "Hits" column does not relate to call counting. it is only the number of samples taken that includes the method.
You may now want to measure with #3387 on top, which should delete the method almost entirely from the samples.

@fedejeanne
Copy link
Contributor

You may now want to measure with #3387 on top, which should delete the method almost entirely from the samples.

Just for future reference, I tried it and you were right: the methods went all the way down in the hotspots --> #3387 (comment)

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.

[Performance] Full build takes more time since 2024-09 (type inference)
3 participants