TwicPics / components

A Web component library that brings the power of TwicPics to your favorite web framework.
MIT License
53 stars 2 forks source link

React Native - VisibilityDetector.measure() causes `Excessive number of pending callbacks` on low spec Android devices #103

Closed ChrisYohann closed 1 month ago

ChrisYohann commented 1 month ago

Hello there,

I am currently using react-native components library in an app that I'm developing. Basically, I have a react-native FlatList that renders a Component which is a thumbnail (using TwicImg) with some additional informations but just <View> and <Text> components.

Recently, I've experienced some performance issues when running my app on Android (emulator and 2017 Samsung Galaxy S8). When I navigate between screens that render TwicImg components, sometimes (it's hard to reproduce consistently), I have this error :

Warning: Please report: Excessive number of pending callbacks: 501. Some pending callbacks that might have leaked by never being called from native code:
{"1855":{},"1856":{},"1857":{},"1858":{},"1859":{},"1860":{},"1861":{},"1862":{},"1863":{},"1864":{},"1865":{},"1866":{},"1867":{},"1868":{},"1869":{},"1870":{},"1871":{},"1872":{},"1873":{},"1874":{},"1875":{},"1876":{},"1877":{},"1878":{},"1879":{},"1880":{},"1881":{},"1882":{},"1883":{},"1884":{},"1885":{},"1886":{},"1887":{},"1888":{},"1889":{},"1890":{},"1891":{},"1892":{},"1893":{},"1894":{},"1895":{},"1896":{},"1897":{},"1898":{},"1899":{},"1900":{},"1901":{},"1902":{},"...(truncated keys)...":451}

From my research, as it says, this warning is due to asynchronous callbacks that are stacking up again and again, hence leading to the JS thread being saturated.

It was really hard to track this issue down since the code is minified and obfuscated, but after building it from the source and disabling terser minification / formatting options, I managed to track down where the problem comes from.

The Visibility Detector component has a useEffect that waits until the media has reached the viewport (if eager prop is not set to true of course) to change the visibility of the media.

https://github.com/TwicPics/components/blob/main/src/react-native/visibilityDetector.tsx#L74-L83

    // eslint-disable-next-line consistent-return
    useEffect(() => {
        if (eager) {
            onVisibilityChanged(true);
        } else {
            observe();
            return unobserve;
        }
    }, []);

Problem is, the observe function sets an interval with setInterval to check the media position relative to the viewport. The default interval for this call is 100ms, no matter the execution time of the measure function. If the measure execution time is greater than the interval, callbacks are stacked and this leads to my issue below.

It basically happens where I have a list of some items which are mounted (so the useEffect has been triggered), but the image is not visible to the screen because I don't scroll enough to see it. So the measurement is done again and again. To make sure the error was from there, I added some logs to measure the execution of the measure function. On IOS, it takes few ms to be triggered, but on low spec Android devices, it can take up to 600-700ms.

I might set the eager prop to true as a workaround, and I know that I can also increase the measurementInterval but I think that the main issue is the use of setInterval, which schedule a call regardless of the previous measurement completion. I think a setTimeout at the end of the measure function will ensure that the measurement will be performed measurementInterval ms AFTER the last execution, thus preventing the callbacks to be stacked.

I'm using react-native 0.72.10 with expo sdk 49.

Great job by the way !

Cheers,

Chris

mbgspcii commented 1 month ago

Hi @ChrisYohann.

Thank you for your feedback and insightful analysis.

I think using setTimeOut is a good idea.

We'll test it out and get back to you with an update.

Thanks again++

Miguel

mbgspcii commented 1 month ago

Hi @ChrisYohann Some news: we were able to reproduce the anomaly : image

The new version is currently being tested and, for the time being, doesn't seem to be causing any problems.

If all goes well, it should be available tomorrow or thursday.

I'll confirm here the production launch.

Thanks again and have a nice evening.

mbgspcii commented 1 month ago

Hi @ChrisYohann

We've just released version 0.29.2, which should solve your problem.

Don't hesitate to keep us informed.

Thanks again for opening this issue.