Conversation
Why === `TextAttributes` is copied into every `AttributedString` fragment and is part of every text measure cache key so its memory usage adds up. We can reduce the size of its fields by several bytes, saving about 22% on Android and 15% on iOS. Another way to think of this is that it frees up a memory budget to add support for features like inheritable styles. How === The enum types in `TextAttributes` use `uint8_t` (1 byte) where possible. There are some exceptions like `FontWeight` which uses `uint16_t` because it has values up to 900, and `FontVariant` stays `int`. Also, there is an optimization around the ordering of font-related fields to reduce byte-alignment gaps by ordering all the 4-byte fields first, followed by 2-byte optionals. This is small and saves 16 bytes on iOS and 8 bytes on Android, but is also a win just from reordering a few fields that are close to each other. There is a test case to keep this optimization in place. | Platform | Before | After | | ------------- | --------- | --------- | | Android | 256 bytes | 200 bytes | | iOS | 376 bytes | 320 bytes | I also checked that `-O3` didn't do these optimizations already. Test Plan === Fantom tests and the `attributedstring` gtests.
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
|
@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D122902861. |
Contributor
|
Can you please update the .api file? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
TextAttributesis copied into everyAttributedStringfragment and is part of every text measure cache key so its memory usage adds up. We can reduce the size of its fields by several bytes, saving about 22% on Android and 15% on iOS. Another way to think of this is that it frees up a memory budget to add support for features like inheritable styles.The enum types in
TextAttributesuseuint8_t(1 byte) where possible. There are some exceptions likeFontWeightwhich usesuint16_tbecause it has values up to 900, andFontVariantstaysint.Also, there is an optimization around the ordering of font-related fields to reduce byte-alignment gaps by ordering all the 4-byte fields first, followed by 2-byte optionals. This is small and saves 16 bytes on iOS and 8 bytes on Android, but is also a win just from reordering a few fields that are close to each other. There is a test case to keep this optimization in place.
I also checked that
-O3didn't do these optimizations already.Changelog:
[INTERNAL] [CHANGED] - Shrink each TextAttributes instance by 56 bytes
Test Plan:
Fantom tests and the
attributedstringgtests.