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: memoization #291

Open
andreialecu opened this issue Jun 25, 2021 · 3 comments
Open

Performance: memoization #291

andreialecu opened this issue Jun 25, 2021 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@andreialecu
Copy link
Contributor

andreialecu commented Jun 25, 2021

Hi again.

While fixing the other issue yesterday, it got me thinking that trying to determine which font to use in each render might not be super efficient.

I decided to take a look at it from a performance perspective, here are my findings.

It appears that on an Oppo A72 Android device, it takes about 1ms to execute the logic in the overridden render function. Now, if there are 16 Text nodes on the screen, that's already 16 ms, so a dropped frame.

Screenshot 2021-06-25 at 10 15 34

If however I add some memoization, it drops to 0.1ms most of the time, which is a 10x improvement:

Screenshot 2021-06-25 at 10 19 31

I'll open a PR shortly with this for your consideration

@andreialecu
Copy link
Contributor Author

Actually my second screenshot is wrong, it's fast because the useMemo is missing a dependency on element.

But I have further narrowed down the timing, and it's actually generateOverrideStyle that takes 1+ms each time.

Screenshot 2021-06-25 at 10 42 43

Since it only works with fontFamily, fontWeight, and fontStyle and those will be fairly static, it should be possible to cache the result.

@mthines
Copy link

mthines commented Jul 8, 2021

Just FYI: @andreialecu I also experienced bad performance using this library, thought it was waay to big for the task, so I've created my own library.

https://www.npmjs.com/package/@mthines/react-native-font-face
https://github.com/mthines/react-native-font-face

@kylerjensen
Copy link
Owner

@andreialecu Thanks for doing the research on this. I don't have the bandwidth to look into this right now, but I'm happy to accept a PR.

@kylerjensen kylerjensen pinned this issue Nov 24, 2021
@kylerjensen kylerjensen added the help wanted Extra attention is needed label Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants