Skip to content

Added a few performance improvements for ReceiptParser#2124

Merged
NachoSoto merged 2 commits into
mainfrom
receipt-parser-performance
Dec 13, 2022
Merged

Added a few performance improvements for ReceiptParser#2124
NachoSoto merged 2 commits into
mainfrom
receipt-parser-performance

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

See also #2123 and #2107 (comment).

@NachoSoto NachoSoto added the perf label Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids creating an array of N elements just to calculate the sum.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One liner, but also avoids jumps. I don't think the optimizer could have figured this out.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`
// ...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't actually improve performance thanks to Swift's COW behavior, but it cleans up an unnecessary value.

@vegaro vegaro Dec 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 NachoSoto force-pushed the receipt-parser-performance branch from fcbc3d4 to d7fad62 Compare December 13, 2022 18:24
@NachoSoto NachoSoto merged commit 8d6f80b into main Dec 13, 2022
@NachoSoto NachoSoto deleted the receipt-parser-performance branch December 13, 2022 19:08
NachoSoto pushed a commit that referenced this pull request Dec 14, 2022
**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)
@vegaro vegaro added pr:other and removed pr:perf labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants