Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Paywalls V2] Convert Codable structs to classes #4665

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

joshdholtz
Copy link
Member

Motivation

We were getting random-ish EXC_BAD_ACCESS after the merge of the badge PR. It took some debuggin and it turned out..

When decoding 5-ish paywall v2 in an OfferingsResponse, the struct structure while in the encoder would just fail.

Note that this is in the ENCODER... and not the DECODER. So this was happening when we decode string response to Swift object and then encode back for storage.

Description

After some debugging and experimentation with Mark, it was our conclusion that our OfferingsResponse struct with the Paywalls V2 struct were causing some sort of struct structure that was WAY too big causing some sort of buffer overflows or something.

We experiment with converting one struct to a class and it made the issue go away. So we continue this in the PR to convert all Paywalls V2 components that are codable from structs (value type) to classes (reference type) to mitigate this issue.

Copy link

emerge-tools bot commented Jan 14, 2025

1 build decreased size

Name Version Download Change Install Change Approval
Paywalls
com.revenuecat.PaywallsTester
1.0 (1) 10.5 MB ⬇️ 1.2 MB (-10.23%) 39.3 MB ⬇️ 3.8 MB (-8.95%) N/A

Paywalls 1.0 (1)
com.revenuecat.PaywallsTester

⚖️ Compare build
📦 Install build
⏱️ Analyze build performance

Total install size change: ⬇️ 3.8 MB (-8.95%)
Total download size change: ⬇️ 1.2 MB (-10.23%)

Largest size changes

Item Install Size Change
RevenueCat.OfferingsFactory.OfferingsFactory ⬇️ -108.9 kB
Code Signature ⬇️ -96.9 kB
RevenueCatUI.IconComponentViewModel.IconComponentViewModel ⬇️ -70.2 kB
RevenueCatUI.ImageComponentViewModel.ImageComponentViewModel ⬇️ -69.3 kB
DYLD.String Table ⬆️ 53.2 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

Comment on lines +81 to +89
let visible = other?.visible ?? base?.visible
let baseUrl = other?.baseUrl ?? base?.baseUrl
let iconName = other?.iconName ?? base?.iconName
let formats = other?.formats ?? base?.formats
let size = other?.size ?? base?.size
let padding = other?.padding ?? base?.padding
let margin = other?.margin ?? base?.margin
let color = other?.color ?? base?.color
let iconBackground = other?.iconBackground ?? base?.iconBackground
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to do this because Swift compiler was timing out 🤷‍♂️

@joshdholtz joshdholtz force-pushed the paywalls-v2/move-codable-structs-to-classes branch from 39f6e72 to 0e4fd29 Compare January 14, 2025 21:42
@joshdholtz joshdholtz merged commit 0d6c52f into main Jan 15, 2025
10 checks passed
@joshdholtz joshdholtz deleted the paywalls-v2/move-codable-structs-to-classes branch January 15, 2025 00:49
This was referenced Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants