kirillzyusko / react-native-keyboard-controller

Keyboard manager which works in identical way on both iOS and Android
https://kirillzyusko.github.io/react-native-keyboard-controller/
MIT License
1.78k stars 81 forks source link

perf: lookup method only once #682

Closed kirillzyusko closed 2 weeks ago

kirillzyusko commented 2 weeks ago

📜 Description

In https://github.com/kirillzyusko/react-native-keyboard-controller/pull/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

🤔 How Has This Been Tested?

Tested on CI.

📝 Checklist

github-actions[bot] commented 2 weeks ago

📊 Package size report

Current size Target Size Difference
159580 bytes 159441 bytes 139 bytes 📈
gronxb commented 2 weeks ago

@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.