Concurrent file downloads

[ouchadam]

Adds support for concurrently downloading files within a batch.

When using the library with big batches (2000+ files) the overhead of connecting/handshaking/latency of downloading each file becomes rather large (2000 * 0.5s = 16 minutes), this becomes especially apparent on smaller files which don't have a lot of time to fully saturate the connection.

• Adds DownloadManagerBuilder.withConcurrentFileDownloading defaults to disabled
• Uses the SequentialFilesDownloader for adding completed batches (as I haven't been able to test this)
• Adds a ConcurrentFilesDownloader which uses a fixed thread pool of 4

This change dropped my big batch download times from 16-20 minutes down to 2-3 minutes

[hal9002]

Can one of the admins review this PR?

[ouchadam]

@Mecharyry I wasn't sure if we would want to enable this as the default behaviour in the demo

Here's a snippet if you'd like to test locally

        downloadManager = DownloadManagerBuilder
                .newInstance(this, handler, R.mipmap.ic_launcher_round)
                .withCustomHttpClient(customHttpClient())
                .withLogHandle(new DemoLogHandle())
                .withConcurrentFileDownloading() // enable the feature

...
        // add more files to the batch
        Batch batch = Batch.with(primaryStorageWithDownloadsSubpackage, BATCH_ID_1, "Batch 1 Title")
                .downloadFrom(FIVE_MB_FILE_URL).saveTo("foo/bar", "5mb.zip").withIdentifier(FILE_ID_1).apply()
                .downloadFrom(FIVE_MB_FILE_URL).saveTo("foo/bar", "5.zip.1").withIdentifier(DownloadFileIdCreator.createFrom("5.1")).apply()
                .downloadFrom(FIVE_MB_FILE_URL).saveTo("foo/bar", "5.zip.2").withIdentifier(DownloadFileIdCreator.createFrom("5.2")).apply()
                .downloadFrom(FIVE_MB_FILE_URL).saveTo("foo/bar", "5.zip.3").withIdentifier(DownloadFileIdCreator.createFrom("5.3")).apply()
                .downloadFrom(FIVE_MB_FILE_URL).saveTo("foo/bar", "5.zip.4").withIdentifier(DownloadFileIdCreator.createFrom("5.4")).apply()
                .downloadFrom(FIVE_MB_FILE_URL).saveTo("foo/bar", "5.zip.5").withIdentifier(DownloadFileIdCreator.createFrom("5.5")).apply()
                .downloadFrom(TEN_MB_FILE_URL).apply()
                .downloadFrom(TWENTY_MB_FILE_URL).apply()
            .build();
        downloadManager.download(batch);

The library still suffers from the head requests being made before the batch starts which means there's a delay before the download starts when multiple files are submitted but providing the FileSize upfront fixes this.

[Mecharyry]

I was more thinking to add a checkbox in the demo that allows the current downloads to be handled sequentially or concurrently 😄 but I can do that in a separate PR later 😄

[ouchadam]

ah, that would be helpful but would mean allowing the demo app to reconfigure the DownloadManager instance which it doesn't do yet 😢

[zegnus]

@ouchadam have you tried extensively:

• Pause-resume-delete user events in rapid succession, such as pause, resume, pause, resume, delete
• Network disconnections switch from Wi-Fi, Mobile, Airplane mode, etc..

The demo should be able to provide an ideal environment for these kinds of tests... right now is difficult to validate that the concurrent approach is rock solid as it is in this PR, but it looks good :S

[ouchadam]

@zegnus

We've tested start/delete and network condition changes (wifi/4g/airplane etc), we don't currently have pause functionality so that hasn't been checked from the main app, it works fine within the demo though!

As the change is opt-in I'd argue the risk to existing functionality is low 👻 Once the feature is out in users hands I'll be able to provide more feedback

[Mecharyry]

Sorry @ouchadam for some reason you can't see them on here when the PR is closed. I've changed the config so that it won't close PRs when they fail now.

[ouchadam]

I've pushed the fixes for ./gradlew build

[ouchadam]

what's the status on this? the CI is failing due to out of date dependencies 🦀