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.

Sign up for GitHub

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

Jump to bottom

[core] [java] More micro opts #4417

Merged
merged 12 commits into from
Mar 23, 2023
Merged

[core] [java] More micro opts #4417

merged 12 commits into from
Mar 23, 2023

Conversation

oowekyala
Copy link
Member

@oowekyala oowekyala commented Mar 11, 2023 •
edited
Loading

Describe the PR

  • Use MethodHandle in Attribute instead of Method. This should be good for performance and also allows us to treat a public method of a package-private superclass as an attribute (which fails when using Method).
  • Various other minor improvements

Overall I can measure a consistent ~8% improvement in runtime (on openjdk sources, all java rules, 1 thread).

Related issues

Related to #4353

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@pmd-test
Copy link

pmd-test commented Mar 11, 2023 •
edited
Loading

1 Message
📖 Compared to master:
This changeset changes 10 violations,
introduces 1 new violations, 0 new errors and 0 new configuration errors,
removes 1 violations, 0 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 9 violations,
introduces 2 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 0 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 4 violations,
introduces 2 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 0 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala oowekyala marked this pull request as ready for review March 11, 2023 16:00
@adangel adangel added the for:performance The goal of this change is to improve PMD's performance label Mar 17, 2023
@adangel adangel self-requested a review March 17, 2023 07:26
@adangel adangel changed the title More micro opts [core] [java] More micro opts Mar 17, 2023
@adangel adangel added this to the 7.0.0 milestone Mar 17, 2023
adangel
adangel approved these changes Mar 21, 2023
View reviewed changes
Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

@oowekyala
Also add an optimized forEach impl
c222173 
Benchmark                                   (sourceFname)  Mode  Cnt   Score   Error  Units
StreamBench.foreachOnChildrenStream     /PLSQLParser.java  avgt    4  14,992 ± 0,169  ms/op
StreamBench.foreachOnChildrenStreamOpt  /PLSQLParser.java  avgt    4   8,179 ± 0,141  ms/op
StreamBench.loopOnChildrenStream        /PLSQLParser.java  avgt    4   9,694 ± 0,549  ms/op
StreamBench.optimizedLoop               /PLSQLParser.java  avgt    4   7,631 ± 0,143  ms/op

"foreachOnChildrenStreamOpt" is the new implementation, the old is "foreachOnChildrenStream".
For comparison, this new impl of forEach with a lambda appears slightly faster than using a
for-each loop on the children stream, and using a manually indexed loop (with getChild) is
slightly faster.
@adangel adangel merged commit 919b1d2 into pmd:master Mar 23, 2023
@oowekyala oowekyala deleted the pmd7.micro-opts branch March 23, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adangel adangel adangel approved these changes

Assignees
No one assigned
Labels
for:performance The goal of this change is to improve PMD's performance
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@oowekyala @pmd-test @adangel