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

Use smaller enum types to reduce object sizes. #207

Merged
merged 36 commits into from Jun 7, 2019
Merged

Use smaller enum types to reduce object sizes. #207

merged 36 commits into from Jun 7, 2019

Conversation

bolsinga
Copy link
Contributor

@bolsinga bolsinga commented May 15, 2019

If a model has lots of enums, making them smaller instead of 64-bit NSInteger types will help. Apple only uses 64-bit enums since they need to guarantee binary compatibility for years, even after they add new enum values. Since this is generated source and re-compiled into applications, this does not apply.

Pinterest builds with -Werror -Wconversion. This means the -decodeIntegerForKey: needs a cast to the enumeration type name. Same goes for -initWithModelDictionary:.

This is only applicable to ObjC, since it is the place where these C types will save some space.

Greg Bolsinga and others added 28 commits April 25, 2019 15:48
If a model has lots of enums, making them smaller instead of 64-bit NSInteger types will help. Apple only uses 64-bit enums since they need to guarantee binary compatibility for years, even after they add new enum values. Since this is generated source and re-compiled into applications, this does not apply.

Building the integration models works. However Pinterest builds with -Werror -Wconversion. This means the -decodeIntegerForKey: needs a cast to the enumeration type name. However down in decodeStatement I do not see how to get the enumeration type name for casting.

This is only applicable to ObjC, since it is the place where these C types will save some space.
…tegory to group them together in memory layout.
Thanks!

Co-Authored-By: Jon Parise <jon@indelible.org>
@bolsinga bolsinga changed the title New smaller enums Use smaller enum types to reduce object sizes. May 15, 2019
Sources/Core/ObjCModelRenderer.swift Outdated Show resolved Hide resolved
Sources/Core/ObjectiveCIR.swift Outdated Show resolved Hide resolved
Sources/Core/ObjectiveCIR.swift Show resolved Hide resolved
Tests/CoreTests/ObjectiveCIRTests.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@jparise jparise left a comment

Choose a reason for hiding this comment

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

👍 from me.

@rahul-malik made some good comments, and I defer to him on the final sign-off.

Sources/Core/ObjectiveCIR.swift Outdated Show resolved Hide resolved
@rahul-malik
Copy link
Collaborator

Here is the conflict since GH isn't loading it...

 +    public static final String TYPE = "everything";
++=======
+     public enum EverythingUnsignedCharEnum {
+         UNSIGNED_CHAR_CASE_2(255);
+         private final int value;
+         EverythingUnsignedCharEnum(int value) {
+             this.value = value;
+         }
+         public int getValue() {
+             return this.value;
+         }
+     }
+
+     public enum EverythingUnsignedIntEnum {
+     public enum EverythingUnsignedIntEnum {
+         UNSIGNED_INT_CASE_2(65536);
+         private final int value;
+         EverythingUnsignedIntEnum(int value) {
+             this.value = value;
+         }
+         public int getValue() {
+             return this.value;
+         }
+     }
+
+     public enum EverythingUnsignedShortEnum {
+         CHAR_CASE_2(256);
+         private final int value;
+         EverythingUnsignedShortEnum(int value) {
+             this.value = value;
+         }
+         public int getValue() {
+             return this.value;
+         }
+     }
++>>>>>>> 922ead267767c04087c3c20e43a7323b17bdb8eb```

@bolsinga
Copy link
Contributor Author

Here is the conflict since GH isn't loading it...

 +    public static final String TYPE = "everything";
++=======
+     public enum EverythingUnsignedCharEnum {
+         UNSIGNED_CHAR_CASE_2(255);
+         private final int value;
+         EverythingUnsignedCharEnum(int value) {
+             this.value = value;
+         }
+         public int getValue() {
+             return this.value;
+         }
+     }
+
+     public enum EverythingUnsignedIntEnum {
+     public enum EverythingUnsignedIntEnum {
+         UNSIGNED_INT_CASE_2(65536);
+         private final int value;
+         EverythingUnsignedIntEnum(int value) {
+             this.value = value;
+         }
+         public int getValue() {
+             return this.value;
+         }
+     }
+
+     public enum EverythingUnsignedShortEnum {
+         CHAR_CASE_2(256);
+         private final int value;
+         EverythingUnsignedShortEnum(int value) {
+             this.value = value;
+         }
+         public int getValue() {
+             return this.value;
+         }
+     }
++>>>>>>> 922ead267767c04087c3c20e43a7323b17bdb8eb```

Everything is additive, and the code changes both add code to the same location in the generated file. Is there a proper way to rebase this diff on top of master to get those TYPE changes?

@rahul-malik
Copy link
Collaborator

Doesn't make integration_test regenerate the file for you?

@bolsinga
Copy link
Contributor Author

bolsinga commented Jun 3, 2019

Doesn't make integration_test regenerate the file for you?

After running that, my repository is clean. I do not know how to proceed here. I verified that this branch is up-to-date with origin/master (it has bdd00b0). I run all the make commands and everything appears to be clean.

What code currently (or used to) generate the following? I can't seem to figure this out.

public static final String TYPE = "everything";

@bolsinga
Copy link
Contributor Author

bolsinga commented Jun 5, 2019

Doesn't make integration_test regenerate the file for you?

After running that, my repository is clean. I do not know how to proceed here. I verified that this branch is up-to-date with origin/master (it has bdd00b0). I run all the make commands and everything appears to be clean.

What code currently (or used to) generate the following? I can't seem to figure this out.

public static final String TYPE = "everything";

aha i needed to merge an upstream branch, not my fork's master. :|

@rahul-malik rahul-malik merged commit d3c6322 into pinterest:master Jun 7, 2019
@bolsinga bolsinga deleted the NewSmallerEnums branch June 7, 2019 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants