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

perf: lookup method only once #682

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Nov 12, 2024

📜 Description

In #681 we started to use reflection for detection the amount of params. But reflection is a slow mechanism, so in this PR I'm optimizing lookup stage by calculating a correct method only one time.

💡 Motivation and Context

Lookup can be up to 10-100 times slower comparing to the direct function call. If we call this method too frequently, we can encounter a bottleneck. To make this code faster I decided to cache the method (thus we do a lookup only one time).

This approach should almost neglect performance impact for a lookup stage. The call stage (method invocation) still will add an overhead, but it should have a small impact.

📢 Changelog

Android

  • cache method (make it lazy calculated);
  • move constants to companion object.

🤔 How Has This Been Tested?

Tested on CI.

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko added 🤖 android Android specific 🚀 optimization You optimize something and it becomes working faster labels Nov 12, 2024
@kirillzyusko kirillzyusko self-assigned this Nov 12, 2024
Copy link
Contributor

github-actions bot commented Nov 12, 2024

📊 Package size report

Current size Target Size Difference
159580 bytes 159441 bytes 139 bytes 📈

@gronxb
Copy link
Contributor

gronxb commented Nov 13, 2024

@kirillzyusko

https://github.com/kirillzyusko/react-native-keyboard-controller/pull/681/files#diff-e4529d4a070066ef895b72de7f05df1df3940a0c4fed960bcf0d27d9a7114651L146-R147

https://github.com/kirillzyusko/react-native-keyboard-controller/pull/681/files#diff-e4529d4a070066ef895b72de7f05df1df3940a0c4fed960bcf0d27d9a7114651L151-R154

Thank you for the refactoring! I noticed a small oversight. In that code, the second argument in the eventDispatcher?.let scope should be it instead of eventDispatcher for it to be correct.

@kirillzyusko kirillzyusko marked this pull request as ready for review November 13, 2024 13:14
@kirillzyusko kirillzyusko merged commit 1e48494 into main Nov 13, 2024
16 checks passed
@kirillzyusko kirillzyusko deleted the perf/lookup-method-only-once branch November 13, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific 🚀 optimization You optimize something and it becomes working faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants