From a2283f0171cffb79c423e51fdfec4a5e2fd4959b Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Sat, 30 Jul 2022 20:01:18 -0300 Subject: [PATCH] [#449] Use CompactBlock Streamer download instead of batch downloads (#451) * [#449] Use CompactBlock Streamer download instead of batch downloade This commit implements a small buffer for the stream download operation so it does not store a block at a time and does it in batches instead. Closes #449 * Fix tests * PR Suggestions --- .../CompactBlockDownloadOperation.swift | 41 ++++++++++++++++++- .../Processor/CompactBlockProcessor.swift | 8 ++-- Tests/DarksideTests/AdvancedReOrgTests.swift | 2 +- .../BlockScanOperationTests.swift | 1 + Tests/NetworkTests/BlockStreamingTest.swift | 2 + 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift index d44efcb7..8ecf6516 100644 --- a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift +++ b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift @@ -55,12 +55,30 @@ class CompactBlockStreamDownloadOperation: ZcashOperation { private var cancelable: CancellableCall? private var startHeight: BlockHeight? private var targetHeight: BlockHeight? + private var blockBufferSize: Int + private var buffer: [ZcashCompactBlock] = [] private weak var progressDelegate: CompactBlockProgressDelegate? + /// Creates an Compact Block Stream Download Operation Operation + /// - Parameters: + /// - service: instance that conforms to `LightWalletService` + /// - storage: instance that conforms to `CompactBlockStorage` + /// - blockBufferSize: the number of blocks that the stream downloader will store in memory + /// before writing them to disk. Making this number smaller makes the downloader easier on RAM + /// memory while being less efficient on disk writes. Making it bigger takes up more RAM memory + /// but is less straining on Disk Writes. Too little or too big buffer will make this less efficient. + /// - startHeight: the height this downloader will start downloading from. If `nil`, + /// it will start from the latest height found on the local cacheDb + /// - targetHeight: the upper bound for this stream download. If `nil`, the + /// streamer will call `service.latestBlockHeight()` + /// - progressDelegate: Optional delegate to report ongoing progress conforming to + /// `CompactBlockProgressDelegate` + /// required init( service: LightWalletService, storage: CompactBlockStorage, + blockBufferSize: Int, startHeight: BlockHeight? = nil, targetHeight: BlockHeight? = nil, progressDelegate: CompactBlockProgressDelegate? = nil @@ -70,6 +88,7 @@ class CompactBlockStreamDownloadOperation: ZcashOperation { self.startHeight = startHeight self.targetHeight = targetHeight self.progressDelegate = progressDelegate + self.blockBufferSize = blockBufferSize super.init() self.name = "Download Stream Operation" } @@ -96,7 +115,12 @@ class CompactBlockStreamDownloadOperation: ZcashOperation { case .success(let result): switch result { case .success: - self?.done = true + do { + try self?.flush() + self?.done = true + } catch { + self?.fail(error: error) + } return case .error(let e): self?.fail(error: e) @@ -111,7 +135,7 @@ class CompactBlockStreamDownloadOperation: ZcashOperation { } handler: {[weak self] block in guard let self = self else { return } do { - try self.storage.insert(block) + try self.cache(block, flushCache: false) } catch { self.fail(error: error) } @@ -136,6 +160,19 @@ class CompactBlockStreamDownloadOperation: ZcashOperation { self.cancelable?.cancel() super.cancel() } + + func cache(_ block: ZcashCompactBlock, flushCache: Bool) throws { + self.buffer.append(block) + + if flushCache || buffer.count >= blockBufferSize { + try flush() + } + } + + func flush() throws { + try self.storage.write(blocks: self.buffer) + self.buffer.removeAll(keepingCapacity: true) + } } class CompactBlockBatchDownloadOperation: ZcashOperation { diff --git a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift index 44909277..04392989 100644 --- a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift +++ b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift @@ -227,7 +227,7 @@ public class CompactBlockProcessor { public var maxBackoffInterval = ZcashSDK.defaultMaxBackOffInterval public var rewindDistance = ZcashSDK.defaultRewindDistance public var walletBirthday: BlockHeight - + public private(set) var downloadBufferSize: Int = 10 private(set) var network: ZcashNetwork private(set) var saplingActivation: BlockHeight @@ -624,14 +624,12 @@ public class CompactBlockProcessor { self.backoffTimer = nil let cfg = self.config - - let downloadBlockOperation = CompactBlockBatchDownloadOperation( + let downloadBlockOperation = CompactBlockStreamDownloadOperation( service: self.service, storage: self.storage, + blockBufferSize: self.config.downloadBufferSize, startHeight: range.lowerBound, targetHeight: range.upperBound, - batchSize: self.config.downloadBatchSize, - maxRetries: self.config.retries, progressDelegate: self ) diff --git a/Tests/DarksideTests/AdvancedReOrgTests.swift b/Tests/DarksideTests/AdvancedReOrgTests.swift index 2c42354d..7b0e2050 100644 --- a/Tests/DarksideTests/AdvancedReOrgTests.swift +++ b/Tests/DarksideTests/AdvancedReOrgTests.swift @@ -452,7 +452,7 @@ class AdvancedReOrgTests: XCTestCase { afterReorgSync.fulfill() }, error: self.handleError) - wait(for: [reorgExpectation, afterReorgSync], timeout: 15) + wait(for: [reorgExpectation, afterReorgSync], timeout: 30) XCTAssertEqual(postReorgVerifiedBalance, preReorgVerifiedBalance) XCTAssertEqual(postReorgTotalBalance, preReorgTotalBalance) diff --git a/Tests/NetworkTests/BlockScanOperationTests.swift b/Tests/NetworkTests/BlockScanOperationTests.swift index 0a1aab2b..39208a01 100644 --- a/Tests/NetworkTests/BlockScanOperationTests.swift +++ b/Tests/NetworkTests/BlockScanOperationTests.swift @@ -185,6 +185,7 @@ class BlockScanOperationTests: XCTestCase { let downloadOperation = CompactBlockStreamDownloadOperation( service: service, storage: storage, + blockBufferSize: 10, startHeight: walletBirthDay.height, targetHeight: walletBirthDay.height + 10000, progressDelegate: self diff --git a/Tests/NetworkTests/BlockStreamingTest.swift b/Tests/NetworkTests/BlockStreamingTest.swift index 8112eff9..57220a82 100644 --- a/Tests/NetworkTests/BlockStreamingTest.swift +++ b/Tests/NetworkTests/BlockStreamingTest.swift @@ -77,6 +77,7 @@ class BlockStreamingTest: XCTestCase { let operation = CompactBlockStreamDownloadOperation( service: service, storage: storage, + blockBufferSize: 10, startHeight: startHeight, progressDelegate: self ) @@ -114,6 +115,7 @@ class BlockStreamingTest: XCTestCase { let operation = CompactBlockStreamDownloadOperation( service: service, storage: storage, + blockBufferSize: 10, startHeight: startHeight, progressDelegate: self )