Skip to content

Commit

Permalink
Print non-extension fields by field number order instead of .proto fi…
Browse files Browse the repository at this point in the history
…le order.

This will affect both JSON and TextFormat.  The serialization order for these two formats is not specified, so switching from `.proto` file order to field number order should not be breaking.

This does not affect the order of extensions, which are always printed last (even if their field numbers interleave with regular fields.

This change appears to speed up JSON encoding by about 15%:

```
name                                           old speed               new speed               delta
BM_LoadAdsDescriptor_Upb<NoLayout>              132MB/s ± 2%            129MB/s ±11%     ~             (p=0.841 n=5+5)
BM_LoadAdsDescriptor_Upb<WithLayout>            118MB/s ± 1%            118MB/s ± 2%     ~             (p=0.841 n=5+5)
BM_LoadAdsDescriptor_Proto2<NoLayout>          62.3MB/s ± 2%           62.4MB/s ± 2%     ~             (p=0.841 n=5+5)
BM_LoadAdsDescriptor_Proto2<WithLayout>        61.0MB/s ± 2%           60.1MB/s ± 8%     ~             (p=0.421 n=5+5)
BM_Parse_Upb_FileDesc<UseArena, Copy>           566MB/s ± 1%            572MB/s ± 2%     ~             (p=0.222 n=5+5)
BM_Parse_Upb_FileDesc<UseArena, Alias>          626MB/s ± 1%            620MB/s ± 3%     ~             (p=0.421 n=5+5)
BM_Parse_Upb_FileDesc<InitBlock, Copy>          572MB/s ± 1%            578MB/s ± 2%     ~             (p=0.421 n=5+5)
BM_Parse_Upb_FileDesc<InitBlock, Alias>         636MB/s ± 1%            635MB/s ± 2%     ~             (p=0.841 n=5+5)
BM_Parse_Proto2<FileDesc, NoArena, Copy>        360MB/s ±12%            345MB/s ±14%     ~             (p=0.841 n=5+5)
BM_Parse_Proto2<FileDesc, UseArena, Copy>       692MB/s ± 2%            691MB/s ± 4%     ~             (p=0.548 n=5+5)
BM_Parse_Proto2<FileDesc, InitBlock, Copy>      709MB/s ± 1%            705MB/s ± 2%     ~             (p=0.841 n=5+5)
BM_Parse_Proto2<FileDescSV, InitBlock, Alias>   642MB/s ± 1%            646MB/s ± 1%     ~             (p=0.690 n=5+5)
BM_SerializeDescriptor_Proto2                  1.19GB/s ± 8%           1.22GB/s ± 8%     ~             (p=0.095 n=5+5)
BM_SerializeDescriptor_Upb                      725MB/s ± 1%            732MB/s ± 1%     ~             (p=0.056 n=5+5)
BM_JsonParse_Upb                                187MB/s ± 1%            188MB/s ± 2%     ~             (p=1.000 n=5+4)
BM_JsonParse_Proto2                            18.7MB/s ± 1%           18.6MB/s ± 2%     ~             (p=0.841 n=5+5)
BM_JsonSerialize_Upb                            106MB/s ± 1%            122MB/s ± 1%  +14.65%          (p=0.008 n=5+5)
BM_JsonSerialize_Proto2                        54.0MB/s ± 2%           53.9MB/s ± 3%     ~             (p=1.000 n=5+5)
```

PiperOrigin-RevId: 619004014
  • Loading branch information
haberman authored and copybara-github committed Mar 26, 2024
1 parent 1211435 commit 8612d5e
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions upb/reflection/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "upb/mini_table/extension.h"
#include "upb/mini_table/field.h"
#include "upb/mini_table/internal/field.h"
#include "upb/mini_table/internal/message.h"
#include "upb/mini_table/message.h"
#include "upb/reflection/def.h"
#include "upb/reflection/def_pool.h"
#include "upb/reflection/message_def.h"
Expand Down Expand Up @@ -121,19 +123,20 @@ void upb_Message_ClearByDef(upb_Message* msg, const upb_MessageDef* m) {
bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m,
const upb_DefPool* ext_pool, const upb_FieldDef** out_f,
upb_MessageValue* out_val, size_t* iter) {
const upb_MiniTable* mt = upb_MessageDef_MiniTable(m);
size_t i = *iter;
size_t n = upb_MessageDef_FieldCount(m);
size_t n = upb_MiniTable_FieldCount(mt);
const upb_MessageValue zero = {0};
UPB_UNUSED(ext_pool);

// Iterate over normal fields, returning the first one that is set.
while (++i < n) {
const upb_FieldDef* f = upb_MessageDef_Field(m, i);
const upb_MiniTableField* field = upb_FieldDef_MiniTable(f);
upb_MessageValue val = upb_Message_GetFieldByDef(msg, f);
const upb_MiniTableField* field = upb_MiniTable_GetFieldByIndex(mt, i);
upb_MessageValue val = upb_Message_GetField(msg, field, zero);

// Skip field if unset or empty.
if (upb_MiniTableField_HasPresence(field)) {
if (!upb_Message_HasFieldByDef(msg, f)) continue;
if (!upb_Message_HasBaseField(msg, field)) continue;
} else {
switch (UPB_PRIVATE(_upb_MiniTableField_Mode)(field)) {
case kUpb_FieldMode_Map:
Expand All @@ -150,7 +153,8 @@ bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m,
}

*out_val = val;
*out_f = f;
*out_f =
upb_MessageDef_FindFieldByNumber(m, upb_MiniTableField_Number(field));
*iter = i;
return true;
}
Expand Down

0 comments on commit 8612d5e

Please sign in to comment.