Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

Http progress listener has serious performance issue on Android #267

Open
fireonmac opened this issue Jun 23, 2022 · 3 comments
Open

Http progress listener has serious performance issue on Android #267

fireonmac opened this issue Jun 23, 2022 · 3 comments

Comments

@fireonmac
Copy link

fireonmac commented Jun 23, 2022

Is your feature request related to a problem? Please describe.
Http.AddListener('progress', (e) => ...) has serious performance issue when downloading large file(about 50mb) on Android device. I used it to show the progress of file download, but it makes my app too slow.

Describe the solution you'd like
It would be great to behave just like it does on IOS

Describe alternatives you've considered
How about limit frequency of emitting progress event only on Android

Additional context
This problem already seemed to be discussed in #195, but it was closed and still remains unfixed.

@jkbz64
Copy link
Contributor

jkbz64 commented Jun 23, 2022

Note: I'm not a maintainer but I did write the initial progress implementation.

iOS implementation uses .default NSURLSession which under the hood most likely uses variable "buffer" size depending on your device/network connection, whenever URLSessionDownloadDelegate calls back the
urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didWriteData bytesWritten: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64) it returns bytesWritten incremented by the "buffer" length (assuming the buffer gets filled every time) and also might potentially postpone the call to the method so it may feel "better".

Currently the android implementation uses fixed buffer of 1024 bytes which will make roughly 100k calls for 100MB file to the progress listener - again, assuming the buffer gets filled everytime

@thomasvidas

Is there any specific reason buffer size of 1024 bytes was chosen? I think typical size for the network packet will be power of two in the range of 512b -> 32kb.
Raising the buffer size to let's say 8192 bytes would reduce number of listener calls by almost ten times on fast connections (where the problem is very apparent) and probably would alleviate the problem without having to go into much more complicated solutions for reasonable sized files. If you have nothing against I can open the PR, doesn't seem like a big deal considering progress isn't usually used for very small files.

@jkbz64
Copy link
Contributor

jkbz64 commented Jun 23, 2022

To dampen the issue I would advise you @fireonmac to make sure you are removing the listeners after you have created them and don't need anymore and/or early exit for the specific files - or simply just do not subscribe multiple times, it will double the number of callbacks.

If you happen to use react or other library/framework which re-render children on component state change (where you might store the number of downloaded bytes), make sure the stored progress change doesn't actually do that!

@nurfgun
Copy link

nurfgun commented Jun 28, 2022

am on the same team with @fireonmac. and i can confirm that we make sure that only one listener is attached at all time and the component containing percentage get refreshed only once in a while with a throttle. Not so much difference it makes thou.
We are currently using a fork of this repo with #195 as a temporary workaround and hope this resolves soon!
Thanks for your great work, @jkbz64 btw.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants