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 optimization by using Method.getParameterCount() where possible #1849

Merged
merged 1 commit into from Dec 31, 2019

Conversation

dreis2211
Copy link
Contributor

@dreis2211 dreis2211 commented Dec 17, 2019

Hi,

as described in #1848 we could make use of Method.getParameterCount() in order to save some overhead from unnecessary cloning caused by Method.getParameterTypes().

As this is my first contribution to Mockito, let me know if I can do something to help.

Let me know what you think.
Cheers,
Christoph

@codecov-io
Copy link

codecov-io commented Dec 17, 2019

Codecov Report

Merging #1849 into release/3.x will increase coverage by <.01%.
The diff coverage is 92.85%.

Impacted file tree graph

@@                Coverage Diff                @@
##             release/3.x    #1849      +/-   ##
=================================================
+ Coverage          86.69%   86.69%   +<.01%     
- Complexity          2505     2506       +1     
=================================================
  Files                314      314              
  Lines               6606     6607       +1     
  Branches             829      829              
=================================================
+ Hits                5727     5728       +1     
  Misses               678      678              
  Partials             201      201
Impacted Files Coverage Δ Complexity Δ
...mockito/internal/invocation/InvocationMatcher.java 97.82% <0%> (ø) 26 <0> (+1) ⬆️
.../mockito/internal/invocation/TypeSafeMatching.java 88.23% <100%> (ø) 12 <3> (ø) ⬇️
...o/internal/stubbing/answers/ReturnsArgumentAt.java 100% <100%> (ø) 27 <0> (ø) ⬇️
...a/org/mockito/internal/util/ObjectMethodsGuru.java 100% <100%> (ø) 9 <0> (ø) ⬇️
...nal/creation/instance/ConstructorInstantiator.java 95.77% <100%> (ø) 36 <9> (ø) ⬇️
...ito/internal/util/reflection/FieldInitializer.java 90.56% <100%> (ø) 18 <0> (ø) ⬇️
...java/org/mockito/internal/exceptions/Reporter.java 93.54% <100%> (+0.02%) 93 <5> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 204f9c1...a114de1. Read the comment docs.

Copy link
Member

@mockitoguy mockitoguy left a comment

Choose a reason for hiding this comment

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

Looks great! I'll merge as soon as the currently built version is published.

@mockitoguy mockitoguy self-assigned this Dec 31, 2019
@mockitoguy mockitoguy changed the title Fixes #1848 : Use Method.getParameterCount() where possible Performance optimization by using Method.getParameterCount() where possible Dec 31, 2019
@mockitoguy mockitoguy merged commit 084e8af into mockito:release/3.x Dec 31, 2019
@TimvdLippe
Copy link
Contributor

After importing this change into Google, I realized that this change is breaking all downstream Android applications. Should we revert this change similarly to #1845 (see full postmortem at https://github.com/mockito/mockito/wiki/Android-Java-8-%60java.time.Duration%60-postmortem ), given that it does not change our public API?

@dreis2211
Copy link
Contributor Author

That's very unfortunate.

@mockitoguy
Copy link
Member

Should we revert this change similarly to #1845

Absolutely.

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.

None yet

4 participants