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 improvement of KCallableImpl.callBy #36

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

k163377
Copy link

@k163377 k163377 commented Oct 22, 2022

// TODO here we can avoid extra arguments copy by allocating an array of precise size if
// varargs are not present and win some performance

This PR is intended to resolve the TODO comments here.
(I am a little unsure if this improvement is really what the comment intended, since I did not understand the meaning of "if vararg are not present"...)

This PR is based on the following
JetBrains/kotlin#4840

Because some processes were difficult to port directly, we omitted the caching of the default argument setting process.


The local benchmark results are as follows

Before:

Benchmark                                 Mode  Cnt  Score   Error   Units
CallByBenchmark.callByLite               thrpt    5  6.996 ± 0.115  ops/us
CallByBenchmark.callByLiteWithLookup     thrpt    5  5.019 ± 0.055  ops/us
CallByBenchmark.callByReflect            thrpt    5  5.868 ± 0.127  ops/us
CallByBenchmark.callByReflectWithLookup  thrpt    5  4.327 ± 0.031  ops/us

After:

Benchmark                                 Mode  Cnt  Score   Error   Units
CallByBenchmark.callByLite               thrpt    5  7.843 ± 0.218  ops/us
CallByBenchmark.callByLiteWithLookup     thrpt    5  5.606 ± 0.041  ops/us
CallByBenchmark.callByReflect            thrpt    5  5.386 ± 0.107  ops/us
CallByBenchmark.callByReflectWithLookup  thrpt    5  4.520 ± 0.121  ops/us

It was faster, or at least not slower.

@k163377
Copy link
Author

k163377 commented Nov 27, 2022

I will draft until the PR here is merged.
JetBrains/kotlin#4840

@k163377 k163377 marked this pull request as draft November 27, 2022 14:08
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.

1 participant