Use references to Objective C classes instead of looking classes up b…

…y name. This should reduce binary size slightly, small performance improvement, and improve linkage by forcing references to all used classes. Note that this maintains backwards compatibility for sources generated by older protoc for the time being. If you want the benefits you will need to recompile your protos with the newer protoc.
protocolbuffers · Jan 15, 2020 · 74956e1 · 74956e1
1 parent 948740b
commit 74956e1
Show file tree

Hide file tree

Showing 31 changed files with 554 additions and 155 deletions.
diff --git a/Makefile.am b/Makefile.am
@@ -687,6 +687,7 @@ objectivec_EXTRA_DIST=                                                       \
   objectivec/Tests/GPBDictionaryTests.m                                      \
   objectivec/Tests/GPBDictionaryTests.pddm                                   \
   objectivec/Tests/GPBExtensionRegistryTest.m                                \
+  objectivec/Tests/GPBMessageTests+ClassNames.m                              \
   objectivec/Tests/GPBMessageTests+Merge.m                                   \
   objectivec/Tests/GPBMessageTests+Runtime.m                                 \
   objectivec/Tests/GPBMessageTests+Serialization.m                           \

diff --git a/objectivec/GPBDescriptor.m b/objectivec/GPBDescriptor.m
@@ -44,7 +44,7 @@
 
 // The addresses of these variables are used as keys for objc_getAssociatedObject.
 static const char kTextFormatExtraValueKey = 0;
-static const char kParentClassNameValueKey = 0;
+static const char kParentClassValueKey = 0;
 static const char kClassNameSuffixKey = 0;
 
 // Utility function to generate selectors on the fly.
@@ -126,6 +126,8 @@ @implementation GPBDescriptor {
   GPBFileSyntax syntax = file.syntax;
   BOOL fieldsIncludeDefault =
       (flags & GPBDescriptorInitializationFlag_FieldsWithDefault) != 0;
+  BOOL usesClassRefs =
+      (flags & GPBDescriptorInitializationFlag_UsesClassRefs) != 0;
 
   void *desc;
   for (uint32_t i = 0; i < fieldCount; ++i) {
@@ -143,6 +145,7 @@ @implementation GPBDescriptor {
     GPBFieldDescriptor *fieldDescriptor =
         [[GPBFieldDescriptor alloc] initWithFieldDescription:desc
                                              includesDefault:fieldsIncludeDefault
+                                               usesClassRefs:usesClassRefs
                                                       syntax:syntax];
     [fields addObject:fieldDescriptor];
     [fieldDescriptor release];
@@ -217,15 +220,19 @@ - (void)setupExtensionRanges:(const GPBExtensionRange *)ranges count:(int32_t)co
   extensionRangesCount_ = count;
 }
 
+- (void)setupContainingMessageClass:(Class)messageClass {
+  objc_setAssociatedObject(self, &kParentClassValueKey,
+                           messageClass,
+                           OBJC_ASSOCIATION_ASSIGN);
+}
+
 - (void)setupContainingMessageClassName:(const char *)msgClassName {
   // Note: Only fetch the class here, can't send messages to it because
   // that could cause cycles back to this class within +initialize if
   // two messages have each other in fields (i.e. - they build a graph).
-  NSAssert(objc_getClass(msgClassName), @"Class %s not defined", msgClassName);
-  NSValue *parentNameValue = [NSValue valueWithPointer:msgClassName];
-  objc_setAssociatedObject(self, &kParentClassNameValueKey,
-                           parentNameValue,
-                           OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+  Class clazz = objc_getClass(msgClassName);
+  NSAssert(clazz, @"Class %s not defined", msgClassName);
+  [self setupContainingMessageClass:clazz];
 }
 
 - (void)setupMessageClassNameSuffix:(NSString *)suffix {
@@ -241,14 +248,7 @@ - (NSString *)name {
 }
 
 - (GPBDescriptor *)containingType {
-  NSValue *parentNameValue =
-      objc_getAssociatedObject(self, &kParentClassNameValueKey);
-  if (!parentNameValue) {
-    return nil;
-  }
-  const char *parentName = [parentNameValue pointerValue];
-  Class parentClass = objc_getClass(parentName);
-  NSAssert(parentClass, @"Class %s not defined", parentName);
+  Class parentClass = objc_getAssociatedObject(self, &kParentClassValueKey);
   return [parentClass descriptor];
 }
 
@@ -487,6 +487,7 @@ - (instancetype)init {
 
 - (instancetype)initWithFieldDescription:(void *)description
                          includesDefault:(BOOL)includesDefault
+                           usesClassRefs:(BOOL)usesClassRefs
                                   syntax:(GPBFileSyntax)syntax {
   if ((self = [super init])) {
     GPBMessageFieldDescription *coreDesc;
@@ -524,12 +525,17 @@ - (instancetype)initWithFieldDescription:(void *)description
 
     // Extra type specific data.
     if (isMessage) {
-      const char *className = coreDesc->dataTypeSpecific.className;
       // Note: Only fetch the class here, can't send messages to it because
       // that could cause cycles back to this class within +initialize if
       // two messages have each other in fields (i.e. - they build a graph).
-      msgClass_ = objc_getClass(className);
-      NSAssert(msgClass_, @"Class %s not defined", className);
+      if (usesClassRefs) {
+        msgClass_ = coreDesc->dataTypeSpecific.clazz;
+      } else {
+        // Backwards compatibility for sources generated with older protoc.
+        const char *className = coreDesc->dataTypeSpecific.className;
+        msgClass_ = objc_getClass(className);
+        NSAssert(msgClass_, @"Class %s not defined", className);
+      }
     } else if (dataType == GPBDataTypeEnum) {
       if ((coreDesc->flags & GPBFieldHasEnumDescriptor) != 0) {
         enumHandling_.enumDescriptor_ =
@@ -561,6 +567,15 @@ - (instancetype)initWithFieldDescription:(void *)description
   return self;
 }
 
+- (instancetype)initWithFieldDescription:(void *)description
+                         includesDefault:(BOOL)includesDefault
+                                  syntax:(GPBFileSyntax)syntax {
+  return [self initWithFieldDescription:description
+                        includesDefault:includesDefault
+                          usesClassRefs:NO
+                                 syntax:syntax];
+}
+
 - (void)dealloc {
   if (description_->dataType == GPBDataTypeBytes &&
       !(description_->flags & GPBFieldRepeated)) {
@@ -957,33 +972,32 @@ @implementation GPBExtensionDescriptor {
   GPBGenericValue defaultValue_;
 }
 
-@synthesize containingMessageClass = containingMessageClass_;
-
-- (instancetype)initWithExtensionDescription:
-        (GPBExtensionDescription *)description {
+- (instancetype)initWithExtensionDescription:(GPBExtensionDescription *)desc
+                               usesClassRefs:(BOOL)usesClassRefs {
   if ((self = [super init])) {
-    description_ = description;
-
-#if defined(DEBUG) && DEBUG
-    const char *className = description->messageOrGroupClassName;
-    if (className) {
-      NSAssert(objc_lookUpClass(className) != Nil,
-               @"Class %s not defined", className);
-    }
-#endif
+    description_ = desc;
+    if (!usesClassRefs) {
+      // Legacy without class ref support.
+      const char *className = description_->messageOrGroupClass.name;
+      if (className) {
+        Class clazz = objc_lookUpClass(className);
+        NSAssert(clazz != Nil, @"Class %s not defined", className);
+        description_->messageOrGroupClass.clazz = clazz;
+      }
 
-    if (description->extendedClass) {
-      Class containingClass = objc_lookUpClass(description->extendedClass);
-      NSAssert(containingClass, @"Class %s not defined",
-               description->extendedClass);
-      containingMessageClass_ = containingClass;
+      const char *extendedClassName = description_->extendedClass.name;
+      if (extendedClassName) {
+        Class clazz = objc_lookUpClass(extendedClassName);
+        NSAssert(clazz, @"Class %s not defined", extendedClassName);
+        description_->extendedClass.clazz = clazz;
+      }
     }
 
     GPBDataType type = description_->dataType;
     if (type == GPBDataTypeBytes) {
       // Data stored as a length prefixed c-string in descriptor records.
       const uint8_t *bytes =
-          (const uint8_t *)description->defaultValue.valueData;
+          (const uint8_t *)description_->defaultValue.valueData;
       if (bytes) {
         uint32_t length;
         memcpy(&length, bytes, sizeof(length));
@@ -998,12 +1012,16 @@ - (instancetype)initWithExtensionDescription:
       // aren't common, we avoid the hit startup hit and it avoid initialization
       // order issues.
     } else {
-      defaultValue_ = description->defaultValue;
+      defaultValue_ = description_->defaultValue;
     }
   }
   return self;
 }
 
+- (instancetype)initWithExtensionDescription:(GPBExtensionDescription *)desc {
+  return [self initWithExtensionDescription:desc usesClassRefs:NO];
+}
+
 - (void)dealloc {
   if ((description_->dataType == GPBDataTypeBytes) &&
       !GPBExtensionIsRepeated(description_)) {
@@ -1055,7 +1073,11 @@ - (BOOL)isPackable {
 }
 
 - (Class)msgClass {
-  return objc_getClass(description_->messageOrGroupClassName);
+  return description_->messageOrGroupClass.clazz;
+}
+
+- (Class)containingMessageClass {
+  return description_->extendedClass.clazz;
 }
 
 - (GPBEnumDescriptor *)enumDescriptor {

diff --git a/objectivec/GPBDescriptor_PackagePrivate.h b/objectivec/GPBDescriptor_PackagePrivate.h
@@ -80,7 +80,11 @@ typedef struct GPBMessageFieldDescription {
   // Name of ivar.
   const char *name;
   union {
-    const char *className;  // Name for message class.
+    // className is deprecated and will be removed in favor of clazz.
+    // kept around right now for backwards compatibility.
+    // clazz is used iff GPBDescriptorInitializationFlag_UsesClassRefs is set.
+    char *className;  // Name of the class of the message.
+    Class clazz;  // Class of the message.
     // For enums only: If EnumDescriptors are compiled in, it will be that,
     // otherwise it will be the verifier.
     GPBEnumDescriptorFunc enumDescFunc;
@@ -123,8 +127,14 @@ typedef NS_OPTIONS(uint8_t, GPBExtensionOptions) {
 typedef struct GPBExtensionDescription {
   GPBGenericValue defaultValue;
   const char *singletonName;
-  const char *extendedClass;
-  const char *messageOrGroupClassName;
+  union {
+    const char *name;
+    Class clazz;
+  } extendedClass;
+  union {
+    const char *name;
+    Class clazz;
+  } messageOrGroupClass;
   GPBEnumDescriptorFunc enumDescriptorFunc;
   int32_t fieldNumber;
   GPBDataType dataType;
@@ -135,6 +145,11 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {
   GPBDescriptorInitializationFlag_None              = 0,
   GPBDescriptorInitializationFlag_FieldsWithDefault = 1 << 0,
   GPBDescriptorInitializationFlag_WireFormat        = 1 << 1,
+
+  // This is used as a stopgap as we move from using class names to class
+  // references. The runtime needs to support both until we allow a
+  // breaking change in the runtime.
+  GPBDescriptorInitializationFlag_UsesClassRefs        = 1 << 2,
 };
 
 @interface GPBDescriptor () {
@@ -168,9 +183,12 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {
       firstHasIndex:(int32_t)firstHasIndex;
 - (void)setupExtraTextInfo:(const char *)extraTextFormatInfo;
 - (void)setupExtensionRanges:(const GPBExtensionRange *)ranges count:(int32_t)count;
-- (void)setupContainingMessageClassName:(const char *)msgClassName;
+- (void)setupContainingMessageClass:(Class)msgClass;
 - (void)setupMessageClassNameSuffix:(NSString *)suffix;
 
+// Deprecated. Use setupContainingMessageClass instead.
+- (void)setupContainingMessageClassName:(const char *)msgClassName;
+
 @end
 
 @interface GPBFileDescriptor ()
@@ -206,7 +224,15 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {
 // description has to be long lived, it is held as a raw pointer.
 - (instancetype)initWithFieldDescription:(void *)description
                          includesDefault:(BOOL)includesDefault
+                           usesClassRefs:(BOOL)usesClassRefs
                                   syntax:(GPBFileSyntax)syntax;
+
+// Deprecated. Equivalent to calling above with `usesClassRefs = NO`.
+- (instancetype)initWithFieldDescription:(void *)description
+                         includesDefault:(BOOL)includesDefault
+                                  syntax:(GPBFileSyntax)syntax;
+
+
 @end
 
 @interface GPBEnumDescriptor ()
@@ -246,8 +272,11 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {
 @property(nonatomic, readonly) GPBWireFormat alternateWireType;
 
 // description has to be long lived, it is held as a raw pointer.
-- (instancetype)initWithExtensionDescription:
-    (GPBExtensionDescription *)description;
+- (instancetype)initWithExtensionDescription:(GPBExtensionDescription *)desc
+                               usesClassRefs:(BOOL)usesClassRefs;
+// Deprecated. Calls above with `usesClassRefs = NO`
+- (instancetype)initWithExtensionDescription:(GPBExtensionDescription *)desc;
+
 - (NSComparisonResult)compareByFieldNumber:(GPBExtensionDescriptor *)other;
 @end
 

diff --git a/objectivec/GPBRuntimeTypes.h b/objectivec/GPBRuntimeTypes.h
@@ -142,3 +142,10 @@ typedef struct GPBExtensionRange {
   /** Exclusive. */
   uint32_t end;
 } GPBExtensionRange;
+
+/**
+ A type to represent a reference to an Objective C class.
+ This is actually an `objc_class` but the runtime headers will not allow us to
+ reference `objc_class`.
+*/
+typedef struct GPBObjcClassReference GPBObjcClassReference;
diff --git a/objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj b/objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj
@@ -35,6 +35,7 @@
 		8BBEA4BB147C729200C4ADB7 /* libProtocolBuffers.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 7461B52E0F94FAF800A0C422 /* libProtocolBuffers.a */; };
 		8BD3981F14BE59D70081D629 /* GPBUnittestProtos.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BD3981E14BE59D70081D629 /* GPBUnittestProtos.m */; };
 		8BF8193514A0DDA600A2C982 /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 1D30AB110D05D00D00671497 /* Foundation.framework */; };
+		8BFF9D1A23AD582300E63E32 /* GPBMessageTests+ClassNames.m in Sources */ = {isa = PBXBuildFile; fileRef = 8BFF9D1923AD582200E63E32 /* GPBMessageTests+ClassNames.m */; };
 		F401DC2D1A8D444600FCC765 /* GPBArray.m in Sources */ = {isa = PBXBuildFile; fileRef = F401DC2B1A8D444600FCC765 /* GPBArray.m */; };
 		F401DC331A8E5C0200FCC765 /* GPBArrayTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F401DC321A8E5C0200FCC765 /* GPBArrayTests.m */; };
 		F40EE4AB206BF8B90071091A /* GPBCompileTest01.m in Sources */ = {isa = PBXBuildFile; fileRef = F40EE488206BF8B00071091A /* GPBCompileTest01.m */; };
@@ -181,6 +182,7 @@
 		8BD3981D14BE54220081D629 /* unittest_enormous_descriptor.proto */ = {isa = PBXFileReference; lastKnownFileType = text; name = unittest_enormous_descriptor.proto; path = ../../src/google/protobuf/unittest_enormous_descriptor.proto; sourceTree = "<group>"; };
 		8BD3981E14BE59D70081D629 /* GPBUnittestProtos.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = GPBUnittestProtos.m; sourceTree = "<group>"; };
 		8BEB5AE01498033E0078BF9D /* GPBRuntimeTypes.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GPBRuntimeTypes.h; sourceTree = "<group>"; };
+		8BFF9D1923AD582200E63E32 /* GPBMessageTests+ClassNames.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "GPBMessageTests+ClassNames.m"; sourceTree = "<group>"; };
 		F401DC2A1A8D444600FCC765 /* GPBArray.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = GPBArray.h; sourceTree = "<group>"; };
 		F401DC2B1A8D444600FCC765 /* GPBArray.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = GPBArray.m; sourceTree = "<group>"; };
 		F401DC321A8E5C0200FCC765 /* GPBArrayTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = GPBArrayTests.m; sourceTree = "<group>"; };
@@ -485,6 +487,7 @@
 				7461B6A30F94FDF800A0C422 /* GPBMessageTests.m */,
 				F4487C821AAF6AB300531423 /* GPBMessageTests+Merge.m */,
 				F4487C741AADF7F500531423 /* GPBMessageTests+Runtime.m */,
+				8BFF9D1923AD582200E63E32 /* GPBMessageTests+ClassNames.m */,
 				F4487C7E1AAF62CD00531423 /* GPBMessageTests+Serialization.m */,
 				F4B51B1D1BBC610700744318 /* GPBObjectiveCPlusPlusTest.mm */,
 				F41C175C1833D3310064ED4D /* GPBPerfTests.m */,
@@ -656,6 +659,7 @@
 			developmentRegion = English;
 			hasScannedForEncodings = 1;
 			knownRegions = (
+				English,
 				en,
 			);
 			mainGroup = 29B97314FDCFA39411CA2CEA /* CustomTemplate */;
@@ -770,6 +774,7 @@
 				8B4248BB1A8C256A00BC1EC6 /* GPBSwiftTests.swift in Sources */,
 				F40EE50C206C06640071091A /* GPBCompileTest25.m in Sources */,
 				F4584D821ECCB52A00803AB6 /* GPBExtensionRegistryTest.m in Sources */,
+				8BFF9D1A23AD582300E63E32 /* GPBMessageTests+ClassNames.m in Sources */,
 				5102DABC1891A073002037B6 /* GPBConcurrencyTests.m in Sources */,
 				F4487C751AADF7F500531423 /* GPBMessageTests+Runtime.m in Sources */,
 				F40EE4AC206BF8B90071091A /* GPBCompileTest02.m in Sources */,