From b375d010bf57a6d673125330ec47f6e6a7e03f5c Mon Sep 17 00:00:00 2001 From: Dave MacLachlan Date: Tue, 9 Apr 2024 09:00:58 -0700 Subject: [PATCH] Optimize startup be reducing selector lookup. Move away from looking up selectors at proto initialization time to doing optimized string compares at implementation resolving time. PiperOrigin-RevId: 623183331 --- objectivec/GPBDescriptor.m | 55 +--- objectivec/GPBDescriptor_PackagePrivate.h | 10 +- objectivec/GPBMessage.m | 314 ++++++++++++++++++---- objectivec/GPBUtilities.m | 9 +- 4 files changed, 266 insertions(+), 122 deletions(-) diff --git a/objectivec/GPBDescriptor.m b/objectivec/GPBDescriptor.m index 9eb4081bac18..6f2fa9b590f0 100644 --- a/objectivec/GPBDescriptor.m +++ b/objectivec/GPBDescriptor.m @@ -51,40 +51,6 @@ - (instancetype)initWithName:(NSString *)name static const char kClassNameSuffixKey = 0; static const char kFileDescriptorCacheKey = 0; -// Utility function to generate selectors on the fly. -static SEL SelFromStrings(const char *prefix, const char *middle, const char *suffix, - BOOL takesArg) { - if (prefix == NULL && suffix == NULL && !takesArg) { - return sel_getUid(middle); - } - const size_t prefixLen = prefix != NULL ? strlen(prefix) : 0; - const size_t middleLen = strlen(middle); - const size_t suffixLen = suffix != NULL ? strlen(suffix) : 0; - size_t totalLen = prefixLen + middleLen + suffixLen + 1; // include space for null on end. - if (takesArg) { - totalLen += 1; - } - char buffer[totalLen]; - if (prefix != NULL) { - memcpy(buffer, prefix, prefixLen); - memcpy(buffer + prefixLen, middle, middleLen); - buffer[prefixLen] = (char)toupper(buffer[prefixLen]); - } else { - memcpy(buffer, middle, middleLen); - } - if (suffix != NULL) { - memcpy(buffer + prefixLen + middleLen, suffix, suffixLen); - } - if (takesArg) { - buffer[totalLen - 2] = ':'; - } - // Always null terminate it. - buffer[totalLen - 1] = 0; - - SEL result = sel_getUid(buffer); - return result; -} - static NSArray *NewFieldsArrayForHasIndex(int hasIndex, NSArray *allMessageFields) __attribute__((ns_returns_retained)); @@ -594,8 +560,6 @@ - (instancetype)initWithName:(const char *)name fields:(NSArray *)fields { for (GPBFieldDescriptor *fieldDesc in fields) { fieldDesc->containingOneof_ = self; } - - caseSel_ = SelFromStrings(NULL, name, "OneOfCase", NO); } return self; } @@ -682,27 +646,9 @@ - (instancetype)initWithFieldDescription:(void *)description coreDesc = description; } description_ = coreDesc; - getSel_ = sel_getUid(coreDesc->name); - setSel_ = SelFromStrings("set", coreDesc->name, NULL, YES); GPBDataType dataType = coreDesc->dataType; BOOL isMessage = GPBDataTypeIsMessage(dataType); - BOOL isMapOrArray = GPBFieldIsMapOrArray(self); - - if (isMapOrArray) { - // map<>/repeated fields get a *Count property (inplace of a has*) to - // support checking if there are any entries without triggering - // autocreation. - hasOrCountSel_ = SelFromStrings(NULL, coreDesc->name, "_Count", NO); - } else { - // It is a single field; it gets has/setHas selectors if... - // - not in a oneof (negative has index) - // - not clearing on zero - if ((coreDesc->hasIndex >= 0) && ((coreDesc->flags & GPBFieldClearHasIvarOnZero) == 0)) { - hasOrCountSel_ = SelFromStrings("has", coreDesc->name, NULL, NO); - setHasSel_ = SelFromStrings("setHas", coreDesc->name, NULL, YES); - } - } // Extra type specific data. if (isMessage) { @@ -719,6 +665,7 @@ - (instancetype)initWithFieldDescription:(void *)description } // Non map<>/repeated fields can have defaults in proto2 syntax. + BOOL isMapOrArray = GPBFieldIsMapOrArray(self); if (!isMapOrArray && includesDefault) { defaultValue_ = ((GPBMessageFieldDescriptionWithDefault *)description)->defaultValue; if (dataType == GPBDataTypeBytes) { diff --git a/objectivec/GPBDescriptor_PackagePrivate.h b/objectivec/GPBDescriptor_PackagePrivate.h index 6d89d3b631a8..07c83dc061e3 100644 --- a/objectivec/GPBDescriptor_PackagePrivate.h +++ b/objectivec/GPBDescriptor_PackagePrivate.h @@ -80,6 +80,10 @@ typedef struct GPBFileDescription { // Describes a single field in a protobuf as it is represented as an ivar. typedef struct GPBMessageFieldDescription { // Name of ivar. + // Note that we looked into using a SEL here instead (which really is just a C string) + // but there is not a way to initialize an SEL with a constant (@selector is not constant) so the + // additional code generated to initialize the value is actually bigger in size than just using a + // C identifier for large apps. const char *name; union { // className is deprecated and will be removed in favor of clazz. @@ -235,7 +239,6 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) { @package const char *name_; NSArray *fields_; - SEL caseSel_; } // name must be long lived. - (instancetype)initWithName:(const char *)name fields:(NSArray *)fields; @@ -245,11 +248,6 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) { @package GPBMessageFieldDescription *description_; GPB_UNSAFE_UNRETAINED GPBOneofDescriptor *containingOneof_; - - SEL getSel_; - SEL setSel_; - SEL hasOrCountSel_; // *Count for map<>/repeated fields, has* otherwise. - SEL setHasSel_; } @end diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m index 76ff50e447b1..194d6d279926 100644 --- a/objectivec/GPBMessage.m +++ b/objectivec/GPBMessage.m @@ -3216,92 +3216,290 @@ static void ResolveIvarSet(__unsafe_unretained GPBFieldDescriptor *field, } } +// Highly optimized routines for determining selector types. +// Meant to only be used by GPBMessage when resolving selectors in +// `+ (BOOL)resolveInstanceMethod:(SEL)sel`. +// These routines are intended to make negative decisions as fast as possible. +GPB_INLINE char GPBFastToUpper(char c) { return (c >= 'a' && c <= 'z') ? (c - 'a' + 'A') : c; } + +GPB_INLINE BOOL GPBIsGetSelForField(const char *selName, GPBFieldDescriptor *descriptor) { + // Does 'selName' == ''? + // selName and have to be at least two characters long (i.e. ('a', '\0')" is the shortest + // selector you can have). + return (selName[0] == descriptor->description_->name[0]) && + (selName[1] == descriptor->description_->name[1]) && + (strcmp(selName + 1, descriptor->description_->name + 1) == 0); +} + +GPB_INLINE BOOL GPBIsSetSelForField(const char *selName, size_t selNameLength, + GPBFieldDescriptor *descriptor) { + // Does 'selName' == 'set:'? + // Do fastest compares up front + const size_t kSetLength = strlen("set"); + // kSetLength is 3 and one for the colon. + if (selNameLength <= kSetLength + 1) { + return NO; + } + if (selName[kSetLength] != GPBFastToUpper(descriptor->description_->name[0])) { + return NO; + } + + // NB we check for "set" and the colon later in this routine because we have already checked for + // starting with "s" and ending with ":" in `+resolveInstanceMethod:` before we get here. + if (selName[0] != 's' || selName[1] != 'e' || selName[2] != 't') { + return NO; + } + + if (selName[selNameLength - 1] != ':') { + return NO; + } + + // Slow path. + size_t nameLength = strlen(descriptor->description_->name); + size_t setSelLength = nameLength + kSetLength + 1; + if (selNameLength != setSelLength) { + return NO; + } + if (strncmp(&selName[kSetLength + 1], descriptor->description_->name + 1, nameLength - 1) != 0) { + return NO; + } + + return YES; +} + +GPB_INLINE BOOL GPBFieldHasHas(GPBFieldDescriptor *descriptor) { + // It gets has/setHas selectors if... + // - not in a oneof (negative has index) + // - not clearing on zero + return (descriptor->description_->hasIndex >= 0) && + ((descriptor->description_->flags & GPBFieldClearHasIvarOnZero) == 0); +} + +GPB_INLINE BOOL GPBIsHasSelForField(const char *selName, size_t selNameLength, + GPBFieldDescriptor *descriptor) { + // Does 'selName' == 'has'? + // Do fastest compares up front. + const size_t kHasLength = strlen("has"); + if (selNameLength <= kHasLength) { + return NO; + } + if (selName[0] != 'h' || selName[1] != 'a' || selName[2] != 's') { + return NO; + } + if (selName[kHasLength] != GPBFastToUpper(descriptor->description_->name[0])) { + return NO; + } + if (!GPBFieldHasHas(descriptor)) { + return NO; + } + + // Slow path. + size_t nameLength = strlen(descriptor->description_->name); + size_t setSelLength = nameLength + kHasLength; + if (selNameLength != setSelLength) { + return NO; + } + + if (strncmp(&selName[kHasLength + 1], descriptor->description_->name + 1, nameLength - 1) != 0) { + return NO; + } + return YES; +} + +GPB_INLINE BOOL GPBIsCountSelForField(const char *selName, size_t selNameLength, + GPBFieldDescriptor *descriptor) { + // Does 'selName' == '_Count'? + // Do fastest compares up front. + if (selName[0] != descriptor->description_->name[0]) { + return NO; + } + const size_t kCountLength = strlen("_Count"); + if (selNameLength <= kCountLength) { + return NO; + } + + if (selName[selNameLength - kCountLength] != '_') { + return NO; + } + + // Slow path. + size_t nameLength = strlen(descriptor->description_->name); + size_t setSelLength = nameLength + kCountLength; + if (selNameLength != setSelLength) { + return NO; + } + if (strncmp(selName, descriptor->description_->name, nameLength) != 0) { + return NO; + } + if (strncmp(&selName[nameLength], "_Count", kCountLength) != 0) { + return NO; + } + return YES; +} + +GPB_INLINE BOOL GPBIsSetHasSelForField(const char *selName, size_t selNameLength, + GPBFieldDescriptor *descriptor) { + // Does 'selName' == 'setHas:'? + // Do fastest compares up front. + const size_t kSetHasLength = strlen("setHas"); + // kSetHasLength is 6 and one for the colon. + if (selNameLength <= kSetHasLength + 1) { + return NO; + } + if (selName[selNameLength - 1] != ':') { + return NO; + } + if (selName[kSetHasLength] != GPBFastToUpper(descriptor->description_->name[0])) { + return NO; + } + if (selName[0] != 's' || selName[1] != 'e' || selName[2] != 't' || selName[3] != 'H' || + selName[4] != 'a' || selName[5] != 's') { + return NO; + } + + if (!GPBFieldHasHas(descriptor)) { + return NO; + } + // Slow path. + size_t nameLength = strlen(descriptor->description_->name); + size_t setHasSelLength = nameLength + kSetHasLength + 1; + if (selNameLength != setHasSelLength) { + return NO; + } + if (strncmp(&selName[kSetHasLength + 1], descriptor->description_->name + 1, nameLength - 1) != + 0) { + return NO; + } + + return YES; +} + +GPB_INLINE BOOL GPBIsCaseOfSelForOneOf(const char *selName, size_t selNameLength, + GPBOneofDescriptor *descriptor) { + // Does 'selName' == 'OneOfCase'? + // Do fastest compares up front. + if (selName[0] != descriptor->name_[0]) { + return NO; + } + const size_t kOneOfCaseLength = strlen("OneOfCase"); + if (selNameLength <= kOneOfCaseLength) { + return NO; + } + if (selName[selNameLength - kOneOfCaseLength] != 'O') { + return NO; + } + + // Slow path. + size_t nameLength = strlen(descriptor->name_); + size_t setSelLength = nameLength + kOneOfCaseLength; + if (selNameLength != setSelLength) { + return NO; + } + if (strncmp(&selName[nameLength], "OneOfCase", kOneOfCaseLength) != 0) { + return NO; + } + if (strncmp(selName, descriptor->name_, nameLength) != 0) { + return NO; + } + return YES; +} + + (BOOL)resolveInstanceMethod:(SEL)sel { const GPBDescriptor *descriptor = [self descriptor]; if (!descriptor) { return [super resolveInstanceMethod:sel]; } - - // NOTE: hasOrCountSel_/setHasSel_ will be NULL if the field for the given - // message should not have has support (done in GPBDescriptor.m), so there is - // no need for checks here to see if has*/setHas* are allowed. ResolveIvarAccessorMethodResult result = {NULL, NULL}; - // See comment about __unsafe_unretained on ResolveIvarGet. - for (__unsafe_unretained GPBFieldDescriptor *field in descriptor->fields_) { - BOOL isMapOrArray = GPBFieldIsMapOrArray(field); - if (!isMapOrArray) { - // Single fields. - if (sel == field->getSel_) { - ResolveIvarGet(field, &result); - break; - } else if (sel == field->setSel_) { - ResolveIvarSet(field, &result); - break; - } else if (sel == field->hasOrCountSel_) { - int32_t index = GPBFieldHasIndex(field); - uint32_t fieldNum = GPBFieldNumber(field); - result.impToAdd = imp_implementationWithBlock(^(id obj) { - return GPBGetHasIvar(obj, index, fieldNum); - }); - result.encodingSelector = @selector(getBool); + const char *selName = sel_getName(sel); + const size_t selNameLength = strlen(selName); + // A setter has a leading 's' and a trailing ':' (e.g. 'setFoo:' or 'setHasFoo:'). + BOOL couldBeSetter = selName[0] == 's' && selName[selNameLength - 1] == ':'; + if (couldBeSetter) { + // See comment about __unsafe_unretained on ResolveIvarGet. + for (__unsafe_unretained GPBFieldDescriptor *field in descriptor->fields_) { + BOOL isMapOrArray = GPBFieldIsMapOrArray(field); + if (GPBIsSetSelForField(selName, selNameLength, field)) { + if (isMapOrArray) { + // Local for syntax so the block can directly capture it and not the + // full lookup. + result.impToAdd = imp_implementationWithBlock(^(id obj, id value) { + GPBSetObjectIvarWithFieldPrivate(obj, field, value); + }); + result.encodingSelector = @selector(setArray:); + } else { + ResolveIvarSet(field, &result); + } break; - } else if (sel == field->setHasSel_) { + } else if (!isMapOrArray && GPBIsSetHasSelForField(selName, selNameLength, field)) { result.impToAdd = imp_implementationWithBlock(^(id obj, BOOL value) { if (value) { [NSException raise:NSInvalidArgumentException format:@"%@: %@ can only be set to NO (to clear field).", [obj class], - NSStringFromSelector(field->setHasSel_)]; + NSStringFromSelector(sel)]; } GPBClearMessageField(obj, field); }); result.encodingSelector = @selector(setBool:); break; - } else { - GPBOneofDescriptor *oneof = field->containingOneof_; - if (oneof && (sel == oneof->caseSel_)) { - int32_t index = GPBFieldHasIndex(field); - result.impToAdd = imp_implementationWithBlock(^(id obj) { - return GPBGetHasOneof(obj, index); - }); - result.encodingSelector = @selector(getEnum); - break; + } + } + } else { + // See comment about __unsafe_unretained on ResolveIvarGet. + for (__unsafe_unretained GPBFieldDescriptor *field in descriptor->fields_) { + BOOL isMapOrArray = GPBFieldIsMapOrArray(field); + if (GPBIsGetSelForField(selName, field)) { + if (isMapOrArray) { + if (field.fieldType == GPBFieldTypeRepeated) { + result.impToAdd = imp_implementationWithBlock(^(id obj) { + return GetArrayIvarWithField(obj, field); + }); + } else { + result.impToAdd = imp_implementationWithBlock(^(id obj) { + return GetMapIvarWithField(obj, field); + }); + } + result.encodingSelector = @selector(getArray); + } else { + ResolveIvarGet(field, &result); } + break; } - } else { - // map<>/repeated fields. - if (sel == field->getSel_) { - if (field.fieldType == GPBFieldTypeRepeated) { + if (!isMapOrArray) { + if (GPBIsHasSelForField(selName, selNameLength, field)) { + int32_t index = GPBFieldHasIndex(field); + uint32_t fieldNum = GPBFieldNumber(field); result.impToAdd = imp_implementationWithBlock(^(id obj) { - return GetArrayIvarWithField(obj, field); + return GPBGetHasIvar(obj, index, fieldNum); }); + result.encodingSelector = @selector(getBool); + break; } else { + GPBOneofDescriptor *oneof = field->containingOneof_; + if (oneof && GPBIsCaseOfSelForOneOf(selName, selNameLength, oneof)) { + int32_t index = GPBFieldHasIndex(field); + result.impToAdd = imp_implementationWithBlock(^(id obj) { + return GPBGetHasOneof(obj, index); + }); + result.encodingSelector = @selector(getEnum); + break; + } + } + } else { + if (GPBIsCountSelForField(selName, selNameLength, field)) { result.impToAdd = imp_implementationWithBlock(^(id obj) { - return GetMapIvarWithField(obj, field); + // Type doesn't matter, all *Array and *Dictionary types support + // -count. + NSArray *arrayOrMap = GPBGetObjectIvarWithFieldNoAutocreate(obj, field); + return [arrayOrMap count]; }); + result.encodingSelector = @selector(getArrayCount); + break; } - result.encodingSelector = @selector(getArray); - break; - } else if (sel == field->setSel_) { - // Local for syntax so the block can directly capture it and not the - // full lookup. - result.impToAdd = imp_implementationWithBlock(^(id obj, id value) { - GPBSetObjectIvarWithFieldPrivate(obj, field, value); - }); - result.encodingSelector = @selector(setArray:); - break; - } else if (sel == field->hasOrCountSel_) { - result.impToAdd = imp_implementationWithBlock(^(id obj) { - // Type doesn't matter, all *Array and *Dictionary types support - // -count. - NSArray *arrayOrMap = GPBGetObjectIvarWithFieldNoAutocreate(obj, field); - return [arrayOrMap count]; - }); - result.encodingSelector = @selector(getArrayCount); - break; } } } + if (result.impToAdd) { const char *encoding = GPBMessageEncodingForSelector(result.encodingSelector, YES); Class msgClass = descriptor.messageClass; diff --git a/objectivec/GPBUtilities.m b/objectivec/GPBUtilities.m index 8bde2dd63eca..622db83c6aff 100644 --- a/objectivec/GPBUtilities.m +++ b/objectivec/GPBUtilities.m @@ -539,16 +539,17 @@ void GPBSetRetainedObjectIvarWithFieldPrivate(GPBMessage *self, GPBFieldDescript // field, and fall back on the default value. The warning below will only // appear in debug, but the could should be changed so the intention is // clear. - NSString *hasSel = NSStringFromSelector(field->hasOrCountSel_); NSString *propName = field.name; NSString *className = self.descriptor.name; + NSString *firstLetterCapitalizedName = [[[className substringToIndex:1] uppercaseString] + stringByAppendingString:[className substringFromIndex:1]]; NSLog(@"warning: '%@.%@ = nil;' is not clearly defined for fields with " @"default values. Please use '%@.%@ = %@' if you want to set it to " - @"empty, or call '%@.%@ = NO' to reset it to it's default value of " + @"empty, or call '%@.has%@ = NO' to reset it to it's default value of " @"'%@'. Defaulting to resetting default value.", className, propName, className, propName, - (fieldType == GPBDataTypeString) ? @"@\"\"" : @"GPBEmptyNSData()", className, hasSel, - field.defaultValue.valueString); + (fieldType == GPBDataTypeString) ? @"@\"\"" : @"GPBEmptyNSData()", className, + firstLetterCapitalizedName, field.defaultValue.valueString); // Note: valueString, depending on the type, it could easily be // valueData/valueMessage. }