Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add message and field name to binary serialization errors #984

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/bundle-size/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ usually do. We repeat this for an increasing number of files.

| code generator | files | bundle size | minified | compressed |
| ------------------- | ----: | ----------: | --------: | ---------: |
| Protobuf-ES | 1 | 126,123 b | 65,653 b | 15,258 b |
| Protobuf-ES | 4 | 128,312 b | 67,161 b | 15,970 b |
| Protobuf-ES | 8 | 131,074 b | 68,932 b | 16,481 b |
| Protobuf-ES | 16 | 141,524 b | 76,913 b | 18,797 b |
| Protobuf-ES | 32 | 169,315 b | 98,931 b | 24,268 b |
| Protobuf-ES | 1 | 126,619 b | 65,868 b | 15,324 b |
| Protobuf-ES | 4 | 128,808 b | 67,376 b | 16,007 b |
| Protobuf-ES | 8 | 131,570 b | 69,147 b | 16,553 b |
| Protobuf-ES | 16 | 142,020 b | 77,128 b | 18,849 b |
| Protobuf-ES | 32 | 169,811 b | 99,146 b | 24,363 b |
| protobuf-javascript | 1 | 334,193 b | 255,820 b | 42,481 b |
| protobuf-javascript | 4 | 360,861 b | 271,092 b | 43,912 b |
| protobuf-javascript | 8 | 382,904 b | 283,409 b | 45,038 b |
Expand Down
12 changes: 6 additions & 6 deletions packages/bundle-size/chart.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions packages/protobuf-test/src/binary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ describe(`binary serialization`, () => {
).toEqual("foo");
});
});
test("error for invalid data", () => {
const msg = create(ScalarValuesMessageSchema, {
uint32Field: -1, // -1 is invalid for a uint
});
expect(() => toBinary(ScalarValuesMessageSchema, msg)).toThrow(
new Error(
"cannot encode field spec.ScalarValuesMessage.uint32_field to binary: invalid uint32: -1",
),
);
});
});

function testBinary<Desc extends DescMessage>(
Expand Down
321 changes: 159 additions & 162 deletions packages/protobuf-test/src/perf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@
// limitations under the License.

import Benchmark from "benchmark";
import { create, fromBinary, toBinary, protoInt64 } from "@bufbuild/protobuf";
import {
create,
fromBinary,
toBinary,
protoInt64,
toJsonString,
fromJsonString,
} from "@bufbuild/protobuf";
import { readFileSync } from "fs";
import { UserSchema } from "./gen/ts/extra/example_pb.js";
import {
Expand Down Expand Up @@ -95,173 +102,163 @@ interface Test {
}

function setupTests(): Test[] {
const tests: Test[] = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a huge change but I'm just switching to generating the benchmark cases instead of having to repeat code.

{
const bytes = readFileSync(
new URL("perf-payload.bin", import.meta.url).pathname,
);
tests.push({
name: `fromBinary perf-payload.bin`,
fn: () => {
fromBinary(PerfMessageSchema, bytes);
},
});
}
{
const desc = UserSchema;
const tinyUser = create(desc, {
active: false,
manager: { active: true },
});
const data = toBinary(desc, tinyUser);
tests.push({
name: `fromBinary tiny example.User (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
},
});
}
{
const desc = UserSchema;
const normalUser = create(desc, {
firstName: "Jane",
lastName: "Doe",
active: true,
manager: { firstName: "Jane", lastName: "Doe", active: false },
locations: ["Seattle", "New York", "Tokyo"],
projects: { foo: "project foo", bar: "project bar" },
});
const data = toBinary(desc, normalUser);
tests.push({
name: `fromBinary normal example.User (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
},
});
}
{
const desc = ScalarValuesMessageSchema;
const message = create(ScalarValuesMessageSchema, {
doubleField: 0.75,
floatField: -0.75,
int64Field: protoInt64.parse(-1),
uint64Field: protoInt64.uParse(1),
int32Field: -123,
fixed64Field: protoInt64.uParse(1),
fixed32Field: 123,
boolField: true,
stringField: "hello world",
bytesField: new Uint8Array([
104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100,
]),
uint32Field: 123,
sfixed32Field: -123,
sfixed64Field: protoInt64.parse(-1),
sint32Field: -1,
sint64Field: protoInt64.parse(-1),
});
const data = toBinary(desc, message);
tests.push({
name: `fromBinary scalar values (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
},
});
}
{
const desc = RepeatedScalarValuesMessageSchema;
const message = create(desc, {
doubleField: [0.75, 0, 1],
floatField: [0.75, -0.75],
int64Field: [protoInt64.parse(-1), protoInt64.parse(-2)],
uint64Field: [protoInt64.uParse(1), protoInt64.uParse(2)],
int32Field: [-123, 500],
fixed64Field: [protoInt64.uParse(1), protoInt64.uParse(99)],
fixed32Field: [123, 999],
boolField: [true, false, true],
stringField: ["hello", "world"],
bytesField: [
new Uint8Array([104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100]),
],
uint32Field: [123, 123],
sfixed32Field: [-123, -123, -123],
sfixed64Field: [
protoInt64.parse(-1),
protoInt64.parse(-2),
protoInt64.parse(100),
],
sint32Field: [-1, -2, 999],
sint64Field: [
protoInt64.parse(-1),
protoInt64.parse(-99),
protoInt64.parse(99),
],
});
const data = toBinary(desc, message);
tests.push({
name: `fromBinary repeated scalar fields (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
},
});
}
{
const desc = MapsMessageSchema;
const message = create(desc, {
strStrField: { a: "str", b: "xx" },
strInt32Field: { a: 123, b: 455 },
strInt64Field: { a: protoInt64.parse(123) },
strBoolField: { a: true, b: false },
strBytesField: {
a: new Uint8Array([
const tests: Test[] = [
{
name: "perf-payload.bin",
desc: PerfMessageSchema,
msg: fromBinary(
PerfMessageSchema,
readFileSync(new URL("perf-payload.bin", import.meta.url).pathname),
),
},
{
name: "tiny example.User",
desc: UserSchema,
msg: create(UserSchema, { active: false, manager: { active: true } }),
},
{
name: "normal example.User",
desc: UserSchema,
msg: create(UserSchema, {
firstName: "Jane",
lastName: "Doe",
active: true,
manager: { firstName: "Jane", lastName: "Doe", active: false },
locations: ["Seattle", "New York", "Tokyo"],
projects: { foo: "project foo", bar: "project bar" },
}),
},
{
name: "scalar values",
desc: ScalarValuesMessageSchema,
msg: create(ScalarValuesMessageSchema, {
doubleField: 0.75,
floatField: -0.75,
int64Field: protoInt64.parse(-1),
uint64Field: protoInt64.uParse(1),
int32Field: -123,
fixed64Field: protoInt64.uParse(1),
fixed32Field: 123,
boolField: true,
stringField: "hello world",
bytesField: new Uint8Array([
104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100,
]),
uint32Field: 123,
sfixed32Field: -123,
sfixed64Field: protoInt64.parse(-1),
sint32Field: -1,
sint64Field: protoInt64.parse(-1),
}),
},
{
name: "repeated scalar values",
desc: RepeatedScalarValuesMessageSchema,
msg: create(RepeatedScalarValuesMessageSchema, {
doubleField: [0.75, 0, 1],
floatField: [0.75, -0.75],
int64Field: [protoInt64.parse(-1), protoInt64.parse(-2)],
uint64Field: [protoInt64.uParse(1), protoInt64.uParse(2)],
int32Field: [-123, 500],
fixed64Field: [protoInt64.uParse(1), protoInt64.uParse(99)],
fixed32Field: [123, 999],
boolField: [true, false, true],
stringField: ["hello", "world"],
bytesField: [
new Uint8Array([
104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100,
]),
],
uint32Field: [123, 123],
sfixed32Field: [-123, -123, -123],
sfixed64Field: [
protoInt64.parse(-1),
protoInt64.parse(-2),
protoInt64.parse(100),
],
sint32Field: [-1, -2, 999],
sint64Field: [
protoInt64.parse(-1),
protoInt64.parse(-99),
protoInt64.parse(99),
],
}),
},
{
name: "map with scalar keys and values",
desc: MapsMessageSchema,
msg: create(MapsMessageSchema, {
strStrField: { a: "str", b: "xx" },
strInt32Field: { a: 123, b: 455 },
strInt64Field: { a: protoInt64.parse(123) },
strBoolField: { a: true, b: false },
strBytesField: {
a: new Uint8Array([
104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100,
]),
},
int32StrField: { 123: "hello" },
int64StrField: { "9223372036854775807": "hello" },
boolStrField: { true: "yes", false: "no" },
strEnuField: { a: 0, b: 1, c: 2 },
int32EnuField: { 1: 0, 2: 1, 0: 2 },
int64EnuField: { "-1": 0, "2": 1, "0": 2 },
}),
},
{
name: "repeated field with 1000 messages",
desc: MessageFieldMessageSchema,
msg: (() => {
const message = create(MessageFieldMessageSchema);
for (let i = 0; i < 1000; i++) {
message.repeatedMessageField.push(
create(MessageFieldMessage_TestMessageSchema),
);
}
return message;
})(),
},
{
name: "map field with 1000 messages",
desc: MapsMessageSchema,
msg: (() => {
const message = create(MapsMessageSchema);
for (let i = 0; i < 1000; i++) {
message.strMsgField[i.toString()] = create(MapsMessageSchema);
}
return message;
})(),
},
].flatMap(({ name, msg, desc }) => {
const bytes = toBinary(desc, msg);
const jsonString = toJsonString(desc, msg);
return [
{
name: `fromBinary ${name}`,
Comment on lines +235 to +237
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual change - we now benchmark fromBinary, fromJsonString, toBinary, and toJsonString for each example message.

fn: () => {
fromBinary(desc, bytes);
},
},
int32StrField: { 123: "hello" },
int64StrField: { "9223372036854775807": "hello" },
boolStrField: { true: "yes", false: "no" },
strEnuField: { a: 0, b: 1, c: 2 },
int32EnuField: { 1: 0, 2: 1, 0: 2 },
int64EnuField: { "-1": 0, "2": 1, "0": 2 },
});
const data = toBinary(desc, message);
tests.push({
name: `fromBinary map with scalar keys and values (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
{
name: `fromJson ${name}`,
fn: () => {
fromJsonString(desc, jsonString);
},
},
});
}
{
const desc = MessageFieldMessageSchema;
const message = create(desc);
for (let i = 0; i < 1000; i++) {
message.repeatedMessageField.push(
create(MessageFieldMessage_TestMessageSchema),
);
}
const data = toBinary(desc, message);
tests.push({
name: `fromBinary repeated field with 1000 messages (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
{
name: `toBinary ${name}`,
fn: () => {
toBinary(desc, msg);
},
},
});
}
{
const desc = MapsMessageSchema;
const message = create(desc);
for (let i = 0; i < 1000; i++) {
message.strMsgField[i.toString()] = create(desc);
}
const data = toBinary(desc, message);
tests.push({
name: `fromBinary map field with 1000 messages (${data.byteLength} bytes)`,
fn: () => {
fromBinary(desc, data);
{
name: `toJson ${name}`,
fn: () => {
toJsonString(desc, msg);
},
},
});
}
];
});
return tests;
}

Expand Down
Loading