protobuf-ts icon indicating copy to clipboard operation
protobuf-ts copied to clipboard

Make field writing order consistent between CODE_SIZE and SPEED

Open jcready opened this issue 2 years ago • 0 comments

When optimizing for CODE_SIZE the MessageType's internalBinaryWrite is (effectively) generated at runtime and will sort the fields by their field number. https://github.com/timostamm/protobuf-ts/blob/ecb16be5328320f6520febf254d0227169bd6617/packages/runtime/src/reflection-binary-writer.ts#L26-L39

However the same sorting is not applied when optimizing for SPEED. It seems like both optimization strategies could be optimized and made consistent if the fields were sorted in the plugin interpreter instead. https://github.com/timostamm/protobuf-ts/blob/ecb16be5328320f6520febf254d0227169bd6617/packages/plugin/src/interpreter.ts#L372-L386

                 result.push(fi);
             }
         }
-        return result;
+        return result.sort((a, b) => a.no - b.no);
     }

I realize this doesn't make any difference in conforming to the protobuf spec, but it would at least keep the written binary consistent between optimization strategies within protobuf-ts. It also means that rearranging the fields of a message inside the proto file (not the field numbers) wouldn't affect the written binary of said message. e.g. these two arrangements of fields would produce identical binary output for the same field values:

message Foo {
  string baz = 1;
  string qux = 2;
}

message Bar {
  string qux = 2;
  string baz = 1;
}

Rearranging of the fields in the proto file might happen if new fields get added where perhaps it might make sense to visually group some of the new fields with the existing fields. Another case where this happens is if we wanted to group all the deprecated fields together at the top or bottom of the message.

jcready avatar Dec 24 '22 12:12 jcready