Avoid PLT calls to the library memcpy() for field, map and array element copies Before/after (android_arm64, -c opt, fasttable): upb_Array_Set, before (17 instructions): ```armasm sub sp, sp, #0x20 stp x3, x30, [sp, #0x8] ldr x9, [x0] mov w8, #0x1 str x2, [sp] ands x10, x9, #0x3 and x9, x9, #0xfffffffffffffff8 cinc x10, x10, ne lsl x11, x1, x10 lsl w8, w8, w10 mov x1, sp add x0, x9, x11 mov x2, x8 bl memcpy@plt ldr x30, [sp, #0x10] add sp, sp, #0x20 ret ``` after (no call, no stack frame): ```armasm ldr x8, [x0] ands x9, x8, #0x3 and x8, x8, #0xfffffffffffffff8 cinc x9, x9, ne lsl x10, x1, x9 cmp x9, #0x2 add x8, x8, x10 b.gt 1f cbnz x9, 2f strb w2, [x8] ret 1:cmp x9, #0x3 b.ne 3f str x2, [x8] ret 2:str w2, [x8] ret 3:stp x2, x3, [x8] ret ``` PiperOrigin-RevId: 998917610
diff --git a/upb/message/array.c b/upb/message/array.c index f8a670a..a4369af 100644 --- a/upb/message/array.c +++ b/upb/message/array.c
@@ -32,7 +32,7 @@ upb_MessageValue ret; const char* data = upb_Array_DataPtr(arr); const int lg2 = UPB_PRIVATE(_upb_Array_ElemSizeLg2)(arr); - memcpy(&ret, data + (i << lg2), 1 << lg2); + UPB_PRIVATE(_upb_Array_CopyElem)(&ret, data + (i << lg2), lg2); return ret; } @@ -52,7 +52,7 @@ UPB_ASSERT(i < upb_Array_Size(arr)); char* data = upb_Array_MutableDataPtr(arr); const int lg2 = UPB_PRIVATE(_upb_Array_ElemSizeLg2)(arr); - memcpy(data + (i << lg2), &val, 1 << lg2); + UPB_PRIVATE(_upb_Array_CopyElem)(data + (i << lg2), &val, lg2); } bool upb_Array_Append(upb_Array* arr, upb_MessageValue val, upb_Arena* arena) {
diff --git a/upb/message/internal/accessors.h b/upb/message/internal/accessors.h index e1af487..f38b5b8 100644 --- a/upb/message/internal/accessors.h +++ b/upb/message/internal/accessors.h
@@ -277,9 +277,13 @@ const void* val) { UPB_ASSERT(!upb_Message_IsFrozen(msg)); UPB_ASSUME(!upb_MiniTableField_IsExtension(f)); - UPB_PRIVATE(_upb_Message_SetPresence)(msg, f); + // Copy the value before setting the presence: the compiler can't tell that + // the store to the presence bits of `msg` doesn't modify `f`, so it would + // read `f` again after it, without what it knows (e.g. from UPB_ASSUME()s) + // about its representation. UPB_PRIVATE(_upb_MiniTableField_DataCopy) (f, UPB_PRIVATE(_upb_Message_MutableDataPtr)(msg, f), val); + UPB_PRIVATE(_upb_Message_SetPresence)(msg, f); } // Returns the extension `e` of `msg` so that its value can be set, creating it
diff --git a/upb/message/internal/array.h b/upb/message/internal/array.h index 59c56f3..6524c7c 100644 --- a/upb/message/internal/array.h +++ b/upb/message/internal/array.h
@@ -67,6 +67,27 @@ return lg2; } +// Copies an element of 1 << lg2 bytes (see _upb_Array_ElemSizeLg2()), without +// calling the library memcpy() for its variable size. +UPB_INLINE void UPB_PRIVATE(_upb_Array_CopyElem)(void* to, const void* from, + size_t lg2) { + switch (lg2) { + case 0: + UPB_MEMCPY_INLINE(to, from, 1); + return; + case 2: + UPB_MEMCPY_INLINE(to, from, 4); + return; + case 3: + UPB_MEMCPY_INLINE(to, from, 8); + return; + case 4: + UPB_MEMCPY_INLINE(to, from, 16); + return; + } + UPB_UNREACHABLE(); +} + UPB_API_INLINE const void* upb_Array_DataPtr(const struct upb_Array* array) { UPB_PRIVATE(_upb_Array_ElemSizeLg2)(array); // Check assertions. return (void*)(array->UPB_ONLYBITS(data) & ~(uintptr_t)_UPB_ARRAY_MASK_ALL);
diff --git a/upb/message/internal/field_data.h b/upb/message/internal/field_data.h index bcf9c34..459e785 100644 --- a/upb/message/internal/field_data.h +++ b/upb/message/internal/field_data.h
@@ -41,16 +41,16 @@ const upb_MiniTableField* f, void* to, const void* from) { switch (UPB_PRIVATE(_upb_MiniTableField_GetRep)(f)) { case kUpb_FieldRep_1Byte: - memcpy(to, from, 1); + UPB_MEMCPY_INLINE(to, from, 1); return; case kUpb_FieldRep_4Byte: - memcpy(to, from, 4); + UPB_MEMCPY_INLINE(to, from, 4); return; case kUpb_FieldRep_8Byte: - memcpy(to, from, 8); + UPB_MEMCPY_INLINE(to, from, 8); return; case kUpb_FieldRep_StringView: { - memcpy(to, from, sizeof(upb_StringView)); + UPB_MEMCPY_INLINE(to, from, sizeof(upb_StringView)); return; } }
diff --git a/upb/message/internal/map.h b/upb/message/internal/map.h index b32cfaa..b048255 100644 --- a/upb/message/internal/map.h +++ b/upb/message/internal/map.h
@@ -99,15 +99,19 @@ } // Avoid emitting an out-of-line memcpy call when the size is not a compile-time -// constant -UPB_FORCEINLINE void* _upb_map_memcpy(void* dst, const void* src, size_t size) { +// constant (UPB_MEMCPY_INLINE() keeps the compiler from merging the cases back +// into one memcpy() of a variable size). +UPB_FORCEINLINE void _upb_map_memcpy(void* dst, const void* src, size_t size) { switch (size) { case 1: - return memcpy(dst, src, 1); + UPB_MEMCPY_INLINE(dst, src, 1); + return; case 4: - return memcpy(dst, src, 4); + UPB_MEMCPY_INLINE(dst, src, 4); + return; case 8: - return memcpy(dst, src, 8); + UPB_MEMCPY_INLINE(dst, src, 8); + return; default: UPB_UNREACHABLE(); }
diff --git a/upb/message/map.c b/upb/message/map.c index 664dde2..e01dce6 100644 --- a/upb/message/map.c +++ b/upb/message/map.c
@@ -95,7 +95,7 @@ uintptr_t intkey; ret = upb_inttable_next(&map->t.inttable, &intkey, &v, (intptr_t*)iter); if (ret) { - memcpy(key, &intkey, map->key_size); + _upb_map_memcpy(key, &intkey, map->key_size); } } if (ret) { @@ -129,7 +129,7 @@ _upb_map_fromkey(upb_strtable_iter_key(&i), &ret, map->key_size); } else { uintptr_t intkey = upb_inttable_iter_key(&map->t.inttable, iter); - memcpy(&ret, &intkey, map->key_size); + _upb_map_memcpy(&ret, &intkey, map->key_size); } return ret; }
diff --git a/upb/port/def.inc b/upb/port/def.inc index cfba74f..6685380 100644 --- a/upb/port/def.inc +++ b/upb/port/def.inc
@@ -88,6 +88,19 @@ #define UPB_HAS_BUILTIN(x) 0 #endif +// UPB_MEMCPY_INLINE(dst, src, n): memcpy() of a compile-time constant number +// of bytes `n`, which the compiler never turns into a call to the library +// memcpy(). A plain memcpy() of a constant size is usually inlined as loads and +// stores too, but the compiler can merge such copies of different sizes (e.g. +// the cases of a switch on the size) into one copy of a variable size, which +// it then can't inline; the merge can even happen before the compiler uses the +// UPB_ASSUME()s about the size. +#if UPB_HAS_BUILTIN(__builtin_memcpy_inline) +#define UPB_MEMCPY_INLINE(dst, src, n) __builtin_memcpy_inline(dst, src, n) +#else +#define UPB_MEMCPY_INLINE(dst, src, n) memcpy(dst, src, n) +#endif + #ifdef __has_extension #define UPB_HAS_EXTENSION(x) __has_extension(x) #else
diff --git a/upb/port/undef.inc b/upb/port/undef.inc index 554dfe3..bfe09de 100644 --- a/upb/port/undef.inc +++ b/upb/port/undef.inc
@@ -83,6 +83,7 @@ #undef UPB_HAS_ATTRIBUTE #undef UPB_HAS_CPP_ATTRIBUTE #undef UPB_HAS_BUILTIN +#undef UPB_MEMCPY_INLINE #undef UPB_HAS_EXTENSION #undef UPB_HAS_FEATURE #undef UPB_XSAN_MEMBER