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

Protobuf doesn't consider identity when serialize/deserialize #279

Closed
ultrasecreth opened this issue Apr 10, 2015 · 2 comments
Closed

Protobuf doesn't consider identity when serialize/deserialize #279

ultrasecreth opened this issue Apr 10, 2015 · 2 comments

Comments

@ultrasecreth
Copy link

Given the following unit test

  //Given
  Country country = Country.newBuilder()
     .setName("Argentina")
     .build();

  Exchange exchange = Exchange.newBuilder()
     .setCountry1(country)
     .setCountry2(country)
     .build();

  //When
  byte[] bytes = exchange.toByteArray();

  //Then
  Exchange exchange1 = Exchange.parseFrom(bytes);

  Country country1 = exchange1.getCountry1();
  Country country2 = exchange1.getCountry2();

  assertThat(country1, sameInstance(country2));

It would be nice if that assertion was true, unfortunatelly it isn't, I wonder why if I set the same nested message several times the serializer doesn't use a pointer to avoid wasting bytes (every time that I add a reference to the country, the resulting byte[] is bigger by the size of the serialized country +2, what I guess is the serialized field number) into the wire and also maintain identity after deserialize.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 10, 2015

Protobuf wire format is very simple (see: https://developers.google.com/protocol-buffers/docs/encoding) and does not support cross links. There is no plan to change the wire format as it breaks compatibility. For your use case if you really want to save a few bytes, you can serialize the county messages in a separate repeated field and make the Exchange message store indices into that repeated field.

@xfxyjwf xfxyjwf closed this as completed Apr 10, 2015
@ultrasecreth
Copy link
Author

Ok, good to know and thanks for your answer 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants