Skip to content

Commit

Permalink
Use references to Objective C classes instead of looking classes up b…
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@dmaclach @thomasvl
dmaclach authored and thomasvl committed Jan 15, 2020
1 parent 948740b commit 74956e1
Show file tree
Hide file tree
Showing 31 changed files with 554 additions and 155 deletions.
1 change: 1 addition & 0 deletions Makefile.am
View file
Expand Up @@ -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 \
Expand Down
98 changes: 60 additions & 38 deletions objectivec/GPBDescriptor.m
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand All @@ -143,6 +145,7 @@ @implementation GPBDescriptor {
GPBFieldDescriptor *fieldDescriptor =
[[GPBFieldDescriptor alloc] initWithFieldDescription:desc
includesDefault:fieldsIncludeDefault
usesClassRefs:usesClassRefs
syntax:syntax];
[fields addObject:fieldDescriptor];
[fieldDescriptor release];
Expand Down Expand Up @@ -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 {
Expand All @@ -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];
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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_ =
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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));
Expand All @@ -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_)) {
Expand Down Expand Up @@ -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 {
Expand Down
41 changes: 35 additions & 6 deletions objectivec/GPBDescriptor_PackagePrivate.h
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 () {
Expand Down Expand Up @@ -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 ()
Expand Down Expand Up @@ -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 ()
Expand Down Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions objectivec/GPBRuntimeTypes.h
View file
Expand Up @@ -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;
5 changes: 5 additions & 0 deletions objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj
View file
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -656,6 +659,7 @@
developmentRegion = English;
hasScannedForEncodings = 1;
knownRegions = (
English,
en,
);
mainGroup = 29B97314FDCFA39411CA2CEA /* CustomTemplate */;
Expand Down Expand Up @@ -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 */,
Expand Down

0 comments on commit 74956e1

Please sign in to comment.