search

jaredh159 / tailwind-react-native-classnames

simple, expressive API for tailwindcss + react-native
2.08k stars 84 forks source link

feat: improve tag function performance #256

Closed liaoliao666 closed 10 months ago

jaredh159 commented 11 months ago

hi there! thanks for contributing!

can you give me a little more information about this PR? were you having a performance issue in one of your apps, and have you verified that this made a difference, or have you benchmarked it before and after to see if there is a noticeable improvement?

I ask because the style fn called out to in the original code is already heavily cached, and doesn't re-parse if it's already seen a utility, so I'm not sure that checking the cache one level higher will have any noticeable impact in the performance.

liaoliao666 commented 11 months ago

The style function will recreate a lots of objects eg. inputs, resolved, dependents, ordered and function call like parseInputs before return the cached styles. And I may call the tag function tw almost hundreds times in a page, so it will save memory during per rerender. I know there may not be a noticeable perf improvement, but it should be worthable to do this in the underlying logic i think.

jaredh159 commented 11 months ago

Yeah, I understand the intuition, and you may be correct. But I'm hesitant to make the change based on just speculating, performance optimizations are notorious for not always aligning with our intuition. I think I would need to see some careful benchmarking showing a significant improvement in perf to add another layer of checking the cache, just for the simplicity of reasoning about the code. If you're up for trying to do that benchmarking, and can document your methodology and the speedups you found in a reproducible way, I'm open to the change.