XITRIX / iTorrent

Torrent client for iOS 16+
MIT License
2.19k stars 203 forks source link

-[Feature]: Multi-line input for adding bulk tracker URLs #269

Closed mahee96 closed 1 year ago

mahee96 commented 1 year ago

Feature Request Reason:

Implementation Details:

Deliverables:

Edit: 1

ScreenShots

mahee96 commented 1 year ago

@XITRIX , A small contribution from my side. Hoping to see this pull request getting merged and pushed in next release.

FYI, I hope the releases are still published to AltStore/Sidestore repositories.

Thanks.

XITRIX commented 1 year ago

Hi, sorry for not responding for so long. Thanks for nice implementation, could you please move EditTextView and EditTextViewController in separate files somewhere in CustomViews folder. After that I'll be glad to merge this PR

mahee96 commented 1 year ago

@XITRIX, sure no problem, I can do that. Since I am actively using iTorrent with this feature, I had noticed that the add logic to process all the trackers provided seems to run on main/UI thread and this is causing the renderer to show 9000+ms when adding huge list with more than 200+ trackers.

I will see how I can offload the task to background thread or at least keep it async from the UI thread or show progress bar when doing add on UI thread in sync mode.

But this nuisance should not stop from using the feature as all the 200+ trackers still get added.

Also the implications of having 200+ active trackers and its hit on performance if any needs to be profiled/tested, before fully rolling out.

mahee96 commented 1 year ago

@XITRIX Moved the UI View and its controller for EditTextView into CustomViews directory as requested. Please do the needful.

image

mahee96 commented 1 year ago

@XITRIX, sure no problem, I can do that. Since I am actively using iTorrent with this feature, I had noticed that the add logic to process all the trackers provided seems to run on main/UI thread and this is causing the renderer to show 9000+ms when adding huge list with more than 200+ trackers.

I will see how I can offload the task to background thread or at least keep it async from the UI thread or show progress bar when doing add on UI thread in sync mode.

But this nuisance should not stop from using the feature as all the 200+ trackers still get added.

Also the implications of having 200+ active trackers and its hit on performance if any needs to be profiled/tested, before fully rolling out.

~~@XITRIX , For this nuisance issue, I will create a separate PR later as it is not a show stopper to use this feature. If the user really wants a long list of trackers (200+) to be added in a bulk, we will inquire more diagnostics info from them if at all the UI delay occurs in such cases.~~

Edit 1: Please find my new commits and comment below on the update, No need for new PR, background processing is now included.

Thanks again.

mahee96 commented 1 year ago

@XITRIX, I have created new commits to offload the bulk of parsing and add logic to background/async. Have also tested it on simulator for inputs more than 25K lines (512 unique trackers). The newly added progress circle also works as expected which works without blocking the UI Thread and doesn't cause fence hang anymore.

I understand that these tests aren't actual use case, since adding more than 150+ trackers is going to hog the router with those many TCP connections and basically slow down the overall download due to contention, but this huge of input was to show that the logic is scalable and can be used in other places in the framework if required.

Test Inputs - tracker entries

trackers_test_512_entries.txt trackers_test_28K_entries.txt

image

@XITRIX, sure no problem, I can do that. Since I am actively using iTorrent with this feature, I had noticed that the add logic to process all the trackers provided seems to run on main/UI thread and this is causing the renderer to show 9000+ms when adding huge list with more than 200+ trackers.

I will see how I can offload the task to background thread or at least keep it async from the UI thread or show progress bar when doing add on UI thread in sync mode.

But this nuisance should not stop from using the feature as all the 200+ trackers still get added.

Also the implications of having 200+ active trackers and its hit on performance if any needs to be profiled/tested, before fully rolling out.

XITRIX commented 1 year ago

Sorry for not updating for a long time, I've added 1 comment, could you please fix it and I'll merge this brunch

Thanks in advance

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

mahee96 commented 1 year ago

I have updated the localizations in e9a4d3e, Thanks!

XITRIX commented 1 year ago

Thanks a lot for your contribution!