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

[java] Improve Java parsing performance #237

Merged
merged 1 commit into from
Apr 11, 2024
Merged

[java] Improve Java parsing performance #237

merged 1 commit into from
Apr 11, 2024

Conversation

alexander-yakushev
Copy link
Member

I've noticed that most of the time spent during running tests is calling source-info from different places.

@alexander-yakushev
Copy link
Member Author

It doesn't seem to outright help with times on CI, but it does on my machine. I think these changes are useful nonetheless.

@alexander-yakushev
Copy link
Member Author

alexander-yakushev commented Apr 11, 2024

@vemv Is there an explicit reason why Eastwood runs in each test job and not together with Kondo and others in a dedicated job?

EDIT: found the answer.

# Eastwood is run for every item in the CI matrix, because its results are sensitive to the code in the runtime,
# so we make the most out of this linter by exercising all profiles, JDK versions, etc.

Still, maybe limiting it to a single Java version at least makes sense?

@alexander-yakushev alexander-yakushev requested a review from vemv April 11, 2024 13:41
@vemv
Copy link
Member

vemv commented Apr 11, 2024

Hi @alexander-yakushev !

Please explain the nature of the changes

Thanks - V

(.getKind ^Element %)))
(mapv #(parse-info % root))
(filterv #(= klass (:class %)))
(first))
Copy link
Member Author

Choose a reason for hiding this comment

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

Because of eagerness, this code invoked parse-info on every element of root even though we only need the first one that matches (= klass (:class %).

(.getKind ^Element %)))
(keep #(parse-info % root))
(filter #(= klass (:class %)))
(first))
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bit better due to laziness, but it will still realize at least a 32-chunk of parse-infos before doing first on them.

@@ -476,8 +476,7 @@
[ns sym]
(when-let [ns (find-ns ns)]
(->> (vals (ns-imports ns))
(map #(member-info (-> ^Class % .getName symbol) sym))
(filter identity)
(keep #(member-info (-> ^Class % .getName symbol) sym))
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is minor.

@vemv
Copy link
Member

vemv commented Apr 11, 2024

Still, maybe limiting it to a single Java version at least makes sense?

Not necessarily as Eastwood detects Java method deprecations - these vary per JDK

Plus, code evaluation can fail for arbitrary reasons - the chosen JDK is an important factor, so more JDKs == more coverage

@vemv vemv merged commit 6cc6656 into master Apr 11, 2024
81 checks passed
@vemv vemv deleted the java-parsing branch April 11, 2024 14:08
@vemv
Copy link
Member

vemv commented Apr 11, 2024

Thanks much for the changes!

Hopefully this will make at least some difference.

I'll want to work on #211 sometime in the year.

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.

2 participants