remove multi-threaded downloading

[AndreyNikiforov]

feat: removed multi-threaded downloading and added deprecation notice to --threads-num parameter #180, #188

[ndbroadbent]

also here "deprecated"

[ndbroadbent]

Sure, I just saw this follow-up comment about the download times: #171 (comment)

So that sounds fine to me!

[menkej]

Fine with me - apart from the typo, but I'm currently running some tests on multithreading, so let's wait some days before we really remove it.

[AndreyNikiforov]

Fine with me - apart from the typo, but I'm currently running some tests on multithreading, so let's wait some days before we really remove it.

Current (1.6.2) multi-threaded downloading implementation has a number of shortcomings that cause a majority of issue reported for the project since I started paying more attention to it:

• deduplication does not work with threads > 1, we changed default to 1 to reduce chances of ppl hitting this issue
• until_found is inconsistent (btw it forces threads == 1), causing failing tests (==false alarms); this is because current pub-sub implementation checks counter at publisher and moving this check to subscriber requires additional back pressure implementation

We did confirm in a number of tests that multi threaded downloading does not improve speed on fast and slow connections. Anybody should be able to repeat tests and validate for themselves using 1.6.2. Here are things to keep in mind while testing multi-threaded vs single-threaded:

• iCloud account should not have any duplicates
• test should not use --unit-found parameter

If we are to fix/re-implement parallel downloading, we'll have to keep the following in mind:

• deduplication should take both in-flight tasks and files on disk into account
• until-found may be really tricky to implement to avoid any unnecessary processing/downloading
• it would be most likely more resource efficient to use event loop-based IO concurrency like asyncio

With all that in mind, I feel it would be better to release this feature as 1.7.0, continue any multi-threaded downloading testing using 1.6.2, and revisit parallel downloading if tests show any promises and we have resources to implement parallel downloading correctly. @menkej will that work for you?

[jmonster]

@AndreyNikiforov was this feature re-added? The cli arg --help still advertises this feature (as does the README). I'm not asking you to put it back, but if it is is supposed to work, it doesn't seems broken

https://github.com/icloud-photos-downloader/icloud_photos_downloader/search?q=threads_num

[AndreyNikiforov]

was this feature re-added?

What makes you think so?

The cli arg --help still advertises this feature (as does the README). I'm not asking you to put it back, but if it is is supposed to work, it doesn't seems broken

What do you mean by advertising? Help in 1.7.2 says that feature is deprecated and will be removed. It is not broken, it is just noop (==does nothing).