Added a few performance improvements for ReceiptParser
#2124
Conversation
| @@ -151,7 +151,10 @@ private extension ASN1ContainerBuilder { | |||
|
|
|||
| if lengthDefinition == .indefinite { | |||
| innerContainers = try buildInternalContainers(payload: data.dropFirst(bytesUsedForLength)) | |||
| let innerContainersOverallLength = innerContainers.map { $0.totalBytesUsed }.reduce(0, +) | |||
| let innerContainersOverallLength = innerContainers | |||
| .lazy | |||
There was a problem hiding this comment.
This avoids creating an array of N elements just to calculate the sum.
There was a problem hiding this comment.
I didn't know about this lazy thing. For whoever is interested this article explains it very well https://www.avanderlee.com/swift/lazy-collections-arrays/
There was a problem hiding this comment.
Since this is not very well known, maybe adding a comment with a explanation would be worth it? Something like, reduce is applied without having to create an intermediate collection with map
| throw BitShiftError.unhandledRange | ||
| } | ||
|
|
||
| return 0b11111111 >> (8 - range) |
There was a problem hiding this comment.
One liner, but also avoids jumps. I don't think the optimizer could have figured this out.
There was a problem hiding this comment.
I like it, but it might be worth it to add a couple examples for clarification?
// If range 1, return `0b1`
// If range 2, return `0b11`
// ...
| let intData = [UInt8](receiptData) | ||
|
|
||
| let asn1Container = try self.containerBuilder.build(fromPayload: ArraySlice(intData)) | ||
| let asn1Container = try self.containerBuilder.build(fromPayload: ArraySlice(receiptData)) |
There was a problem hiding this comment.
I think this doesn't actually improve performance thanks to Swift's COW behavior, but it cleans up an unnecessary value.
There was a problem hiding this comment.
I have no idea so I had to ask, does it matter we are creating a ArraySlice on a Data and we were passing a [UInt8] before? Do you know why we had that intermediate value?
There was a problem hiding this comment.
Data is already a Collection of UInt8 values, so this is equivalent to the previous implementation (and we have good test coverage to ensure that).
NachoSoto
force-pushed
the
receipt-parser-performance
branch
from
fcbc3d4
to
d7fad62
Compare
**This is an automatic release.** ### Bugfixes * Un-deprecate `Purchases.configure(withAPIKey:appUserID:)` and `Purchases.configure(withAPIKey:appUserID:observerMode:)` (#2129) via NachoSoto (@NachoSoto) ### Other Changes * `ReceiptFetcherTests`: refactored tests using `waitUntilValue` (#2144) via NachoSoto (@NachoSoto) * Added a few performance improvements for `ReceiptParser` (#2124) via NachoSoto (@NachoSoto) * `CallbackCache`: fixed reference (#2143) via NachoSoto (@NachoSoto) * `PostReceiptDataOperation`: clarified receipt debug log (#2128) via NachoSoto (@NachoSoto) * `CallbackCache`: avoid exposing internal mutable cache (#2136) via NachoSoto (@NachoSoto) * `CallbackCache`: added assertion for tests to ensure we don't leak callbacks (#2137) via NachoSoto (@NachoSoto) * `NetworkOperation`: made `Atomic` references immutable (#2139) via NachoSoto (@NachoSoto) * `ReceiptParser`: ensure parsing never happens in the main thread (#2123) via NachoSoto (@NachoSoto) * `PostReceiptDataOperation`: also print receipt data with `verbose` logs (#2127) via NachoSoto (@NachoSoto) * `BasePurchasesTests`: detecting and fixing many `DeviceCache` leaks (#2105) via NachoSoto (@NachoSoto) * `StoreKitIntegrationTests`: added test to check applying a promotional offer during subscription (#1588) via NachoSoto (@NachoSoto)
See also #2123 and #2107 (comment).