Issue 157169: Fix for Protobuf issue 136: memoized serialized size of packed fields was invalid
DescriptionFix Issue 136: the memoized serialized size for packed fields may not
be properly set. writeTo() may be invoked without a call to
getSerializedSize(), so the generated serialization methods would
write a length of 0 for non-empty packed fields. Just call
getSerializedSize() at the beginning of writeTo(): although this
means that we may compute the byte size needlessly when there
are no packed fields, in practice, getSerializedSize() will
already have been called - all of the writeTo() wrappers in
AbstractMessageLite invoke it.
Tested: new unittest case in WireFormatTest.java now passes
Patch Set 1 #Patch Set 2 : Remove FileOutputStream import #Patch Set 3 : Send review mail #Patch Set 4 : Initialize the memoized size of packed fields to -1 instead of 0 #Patch Set 5 : May as well generate less code and just call getSerializedSize() once at the beginning of writeTo() #
Total comments: 1
MessagesTotal messages: 7
I guess it's technically the case that a packed field always has non-zero size, but maybe we should initialize the cached sizes to -1 anyway to make it more clear when they haven't been initialized? On Tue, Nov 24, 2009 at 7:04 PM, <jasonh@google.com> wrote: > Reviewers: kenton, > > Description: > Fix Issue 136: the memoized serialized size for packed fields may not > be properly set. writeTo() may be invoked without a call to > getSerializedSize(), so the generated serialization methods would > write a length of 0 for non-empty packed fields. Since the object > is immutable, we know that the memoized size is 0 or set to the > correct size. In the generated code, check to see if the memoized > size is 0, and if so, call getSerializedSize() before actually > emitting the field. > > Tested: new unittest case in WireFormatTest.java now passes > > > > Please review this at http://codereview.appspot.com/157169 > > Affected files: > M java/src/test/java/com/google/protobuf/WireFormatTest.java > M src/google/protobuf/compiler/java/java_primitive_field.cc > > > Index: java/src/test/java/com/google/protobuf/WireFormatTest.java > =================================================================== > --- java/src/test/java/com/google/protobuf/WireFormatTest.java (revision > 246) > +++ java/src/test/java/com/google/protobuf/WireFormatTest.java (working > copy) > @@ -102,6 +102,28 @@ > assertEquals(rawBytes, rawBytes2); > } > > + public void testSerializationPackedWithoutGetSerializedSize() > + throws Exception { > + // Write directly to an OutputStream, without invoking > getSerializedSize() > + // This used to be a bug where the size of a packed field was > incorrect, > + // since getSerializedSize() was never invoked. > + TestPackedTypes message = TestUtil.getPackedSet(); > + > + // Directly construct a CodedOutputStream around the actual > OutputStream, > + // because writeTo() now invokes getSerializedSize(); > + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); > + CodedOutputStream codedOutput = > CodedOutputStream.newInstance(outputStream); > + > + message.writeTo(codedOutput); > + > + codedOutput.flush(); > + > + TestPackedTypes message2 = TestPackedTypes.parseFrom( > + outputStream.toByteArray()); > + > + TestUtil.assertPackedFieldsSet(message2); > + } > + > public void testSerializeExtensionsLite() throws Exception { > // TestAllTypes and TestAllExtensions should have compatible wire > formats, > // so if we serialize a TestAllExtensions then parse it as TestAllTypes > Index: src/google/protobuf/compiler/java/java_primitive_field.cc > =================================================================== > --- src/google/protobuf/compiler/java/java_primitive_field.cc (revision > 246) > +++ src/google/protobuf/compiler/java/java_primitive_field.cc (working > copy) > @@ -384,8 +384,17 @@ > void RepeatedPrimitiveFieldGenerator:: > GenerateSerializationCode(io::Printer* printer) const { > if (descriptor_->options().packed()) { > + // writeTo() may be called without getSerializedSize() ever having > been > + // called, so we may need to compute the length of the packed data. > Since > + // the object is immutable, we know that the memoized size is either > set to > + // 0 (via object initialization) or else it has the correct size from > a > + // previous getSerializedSize() call. If the field is not empty and > the size > + // has not yet been computed, just call getSerializedSize() > printer->Print(variables_, > "if (get$capitalized_name$List().size() > 0) {\n" > + " if ($name$MemoizedSerializedSize == 0) {\n" > + " getSerializedSize();\n" > + " }\n" > " output.writeRawVarint32($tag$);\n" > " output.writeRawVarint32($name$MemoizedSerializedSize);\n" > "}\n" > > >
Sign in to reply to this message.
On Tue, Nov 24, 2009 at 7:40 PM, Kenton Varda <kenton@google.com> wrote: > I guess it's technically the case that a packed field always has non-zero > size, but maybe we should initialize the cached sizes to -1 anyway to make > it more clear when they haven't been initialized? That was a confusing sentence. Change "when they haven't been initialized" to "when getSerializedSize() hasn't been called". > > On Tue, Nov 24, 2009 at 7:04 PM, <jasonh@google.com> wrote: > >> Reviewers: kenton, >> >> Description: >> Fix Issue 136: the memoized serialized size for packed fields may not >> be properly set. writeTo() may be invoked without a call to >> getSerializedSize(), so the generated serialization methods would >> write a length of 0 for non-empty packed fields. Since the object >> is immutable, we know that the memoized size is 0 or set to the >> correct size. In the generated code, check to see if the memoized >> size is 0, and if so, call getSerializedSize() before actually >> emitting the field. >> >> Tested: new unittest case in WireFormatTest.java now passes >> >> >> >> Please review this at http://codereview.appspot.com/157169 >> >> Affected files: >> M java/src/test/java/com/google/protobuf/WireFormatTest.java >> M src/google/protobuf/compiler/java/java_primitive_field.cc >> >> >> Index: java/src/test/java/com/google/protobuf/WireFormatTest.java >> =================================================================== >> --- java/src/test/java/com/google/protobuf/WireFormatTest.java (revision >> 246) >> +++ java/src/test/java/com/google/protobuf/WireFormatTest.java (working >> copy) >> @@ -102,6 +102,28 @@ >> assertEquals(rawBytes, rawBytes2); >> } >> >> + public void testSerializationPackedWithoutGetSerializedSize() >> + throws Exception { >> + // Write directly to an OutputStream, without invoking >> getSerializedSize() >> + // This used to be a bug where the size of a packed field was >> incorrect, >> + // since getSerializedSize() was never invoked. >> + TestPackedTypes message = TestUtil.getPackedSet(); >> + >> + // Directly construct a CodedOutputStream around the actual >> OutputStream, >> + // because writeTo() now invokes getSerializedSize(); >> + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); >> + CodedOutputStream codedOutput = >> CodedOutputStream.newInstance(outputStream); >> + >> + message.writeTo(codedOutput); >> + >> + codedOutput.flush(); >> + >> + TestPackedTypes message2 = TestPackedTypes.parseFrom( >> + outputStream.toByteArray()); >> + >> + TestUtil.assertPackedFieldsSet(message2); >> + } >> + >> public void testSerializeExtensionsLite() throws Exception { >> // TestAllTypes and TestAllExtensions should have compatible wire >> formats, >> // so if we serialize a TestAllExtensions then parse it as >> TestAllTypes >> Index: src/google/protobuf/compiler/java/java_primitive_field.cc >> =================================================================== >> --- src/google/protobuf/compiler/java/java_primitive_field.cc (revision >> 246) >> +++ src/google/protobuf/compiler/java/java_primitive_field.cc (working >> copy) >> @@ -384,8 +384,17 @@ >> void RepeatedPrimitiveFieldGenerator:: >> GenerateSerializationCode(io::Printer* printer) const { >> if (descriptor_->options().packed()) { >> + // writeTo() may be called without getSerializedSize() ever having >> been >> + // called, so we may need to compute the length of the packed data. >> Since >> + // the object is immutable, we know that the memoized size is either >> set to >> + // 0 (via object initialization) or else it has the correct size from >> a >> + // previous getSerializedSize() call. If the field is not empty and >> the size >> + // has not yet been computed, just call getSerializedSize() >> printer->Print(variables_, >> "if (get$capitalized_name$List().size() > 0) {\n" >> + " if ($name$MemoizedSerializedSize == 0) {\n" >> + " getSerializedSize();\n" >> + " }\n" >> " output.writeRawVarint32($tag$);\n" >> " output.writeRawVarint32($name$MemoizedSerializedSize);\n" >> "}\n" >> >> >> >
Sign in to reply to this message.
Done. But it occurs to me that this isn't all that much nicer a solution than just calling getSerializedSize() in writeTo(CodedOutputStream output), except that it only happens for messages with packed fields, rather than all messages. Should this compute only the serialized size of the packed field, so that it doesn't needlessly compute the size of all the unpacked fields? Or does this not really matter? I think all of the wrappers now call getSerializedSize() thanks to r240 (I was tearing my hair out trying to figure out why I could not get a new test which just did ByteArrayOutputStream output = new ByteArrayOutputStream(); message.writeTo(output); would not fail from the open source code, but would if I added the same test to our internal version, until I finally looked at the source). Anyway point being that in most cases getSerializedSize() will have already been called anyway so maybe it doesn't matter too much if we compute the size of the entire message rather than just the packed fields. On 2009/11/25 03:42:18, kenton wrote: > On Tue, Nov 24, 2009 at 7:40 PM, Kenton Varda <mailto:kenton@google.com> wrote: > > > I guess it's technically the case that a packed field always has non-zero > > size, but maybe we should initialize the cached sizes to -1 anyway to make > > it more clear when they haven't been initialized? > > > That was a confusing sentence. Change "when they haven't been initialized" > to "when getSerializedSize() hasn't been called". > > > > > > On Tue, Nov 24, 2009 at 7:04 PM, <mailto:jasonh@google.com> wrote: > > > >> Reviewers: kenton, > >> > >> Description: > >> Fix Issue 136: the memoized serialized size for packed fields may not > >> be properly set. writeTo() may be invoked without a call to > >> getSerializedSize(), so the generated serialization methods would > >> write a length of 0 for non-empty packed fields. Since the object > >> is immutable, we know that the memoized size is 0 or set to the > >> correct size. In the generated code, check to see if the memoized > >> size is 0, and if so, call getSerializedSize() before actually > >> emitting the field. > >> > >> Tested: new unittest case in WireFormatTest.java now passes > >> > >> > >> > >> Please review this at http://codereview.appspot.com/157169 > >> > >> Affected files: > >> M java/src/test/java/com/google/protobuf/WireFormatTest.java > >> M src/google/protobuf/compiler/java/java_primitive_field.cc > >> > >> > >> Index: java/src/test/java/com/google/protobuf/WireFormatTest.java > >> =================================================================== > >> --- java/src/test/java/com/google/protobuf/WireFormatTest.java (revision > >> 246) > >> +++ java/src/test/java/com/google/protobuf/WireFormatTest.java (working > >> copy) > >> @@ -102,6 +102,28 @@ > >> assertEquals(rawBytes, rawBytes2); > >> } > >> > >> + public void testSerializationPackedWithoutGetSerializedSize() > >> + throws Exception { > >> + // Write directly to an OutputStream, without invoking > >> getSerializedSize() > >> + // This used to be a bug where the size of a packed field was > >> incorrect, > >> + // since getSerializedSize() was never invoked. > >> + TestPackedTypes message = TestUtil.getPackedSet(); > >> + > >> + // Directly construct a CodedOutputStream around the actual > >> OutputStream, > >> + // because writeTo() now invokes getSerializedSize(); > >> + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); > >> + CodedOutputStream codedOutput = > >> CodedOutputStream.newInstance(outputStream); > >> + > >> + message.writeTo(codedOutput); > >> + > >> + codedOutput.flush(); > >> + > >> + TestPackedTypes message2 = TestPackedTypes.parseFrom( > >> + outputStream.toByteArray()); > >> + > >> + TestUtil.assertPackedFieldsSet(message2); > >> + } > >> + > >> public void testSerializeExtensionsLite() throws Exception { > >> // TestAllTypes and TestAllExtensions should have compatible wire > >> formats, > >> // so if we serialize a TestAllExtensions then parse it as > >> TestAllTypes > >> Index: src/google/protobuf/compiler/java/java_primitive_field.cc > >> =================================================================== > >> --- src/google/protobuf/compiler/java/java_primitive_field.cc (revision > >> 246) > >> +++ src/google/protobuf/compiler/java/java_primitive_field.cc (working > >> copy) > >> @@ -384,8 +384,17 @@ > >> void RepeatedPrimitiveFieldGenerator:: > >> GenerateSerializationCode(io::Printer* printer) const { > >> if (descriptor_->options().packed()) { > >> + // writeTo() may be called without getSerializedSize() ever having > >> been > >> + // called, so we may need to compute the length of the packed data. > >> Since > >> + // the object is immutable, we know that the memoized size is either > >> set to > >> + // 0 (via object initialization) or else it has the correct size from > >> a > >> + // previous getSerializedSize() call. If the field is not empty and > >> the size > >> + // has not yet been computed, just call getSerializedSize() > >> printer->Print(variables_, > >> "if (get$capitalized_name$List().size() > 0) {\n" > >> + " if ($name$MemoizedSerializedSize == 0) {\n" > >> + " getSerializedSize();\n" > >> + " }\n" > >> " output.writeRawVarint32($tag$);\n" > >> " output.writeRawVarint32($name$MemoizedSerializedSize);\n" > >> "}\n" > >> > >> > >> > > >
Sign in to reply to this message.
You're right, we might as well always call it. On Wed, Nov 25, 2009 at 9:08 AM, <jasonh@google.com> wrote: > Done. But it occurs to me that this isn't all that much nicer a solution > than just calling getSerializedSize() in writeTo(CodedOutputStream > output), except that it only happens for messages with packed fields, > rather than all messages. Should this compute only the serialized size > of the packed field, so that it doesn't needlessly compute the size of > all the unpacked fields? Or does this not really matter? I think all of > the wrappers now call getSerializedSize() thanks to r240 (I was tearing > my hair out trying to figure out why I could not get a new test which > just did > ByteArrayOutputStream output = new ByteArrayOutputStream(); > message.writeTo(output); > would not fail from the open source code, but would if I added the same > test to our internal version, until I finally looked at the source). > Anyway point being that in most cases getSerializedSize() will have > already been called anyway so maybe it doesn't matter too much if we > compute the size of the entire message rather than just the packed > fields. > > > On 2009/11/25 03:42:18, kenton wrote: > >> On Tue, Nov 24, 2009 at 7:40 PM, Kenton Varda >> > <mailto:kenton@google.com> wrote: > > > I guess it's technically the case that a packed field always has >> > non-zero > >> > size, but maybe we should initialize the cached sizes to -1 anyway >> > to make > >> > it more clear when they haven't been initialized? >> > > > That was a confusing sentence. Change "when they haven't been >> > initialized" > >> to "when getSerializedSize() hasn't been called". >> > > > > >> > On Tue, Nov 24, 2009 at 7:04 PM, <mailto:jasonh@google.com> wrote: >> > >> >> Reviewers: kenton, >> >> >> >> Description: >> >> Fix Issue 136: the memoized serialized size for packed fields may >> > not > >> >> be properly set. writeTo() may be invoked without a call to >> >> getSerializedSize(), so the generated serialization methods would >> >> write a length of 0 for non-empty packed fields. Since the object >> >> is immutable, we know that the memoized size is 0 or set to the >> >> correct size. In the generated code, check to see if the memoized >> >> size is 0, and if so, call getSerializedSize() before actually >> >> emitting the field. >> >> >> >> Tested: new unittest case in WireFormatTest.java now passes >> >> >> >> >> >> >> >> Please review this at http://codereview.appspot.com/157169 >> >> >> >> Affected files: >> >> M java/src/test/java/com/google/protobuf/WireFormatTest.java >> >> M src/google/protobuf/compiler/java/java_primitive_field.cc >> >> >> >> >> >> Index: java/src/test/java/com/google/protobuf/WireFormatTest.java >> >> =================================================================== >> >> --- java/src/test/java/com/google/protobuf/WireFormatTest.java >> > (revision > >> >> 246) >> >> +++ java/src/test/java/com/google/protobuf/WireFormatTest.java >> > (working > >> >> copy) >> >> @@ -102,6 +102,28 @@ >> >> assertEquals(rawBytes, rawBytes2); >> >> } >> >> >> >> + public void testSerializationPackedWithoutGetSerializedSize() >> >> + throws Exception { >> >> + // Write directly to an OutputStream, without invoking >> >> getSerializedSize() >> >> + // This used to be a bug where the size of a packed field was >> >> incorrect, >> >> + // since getSerializedSize() was never invoked. >> >> + TestPackedTypes message = TestUtil.getPackedSet(); >> >> + >> >> + // Directly construct a CodedOutputStream around the actual >> >> OutputStream, >> >> + // because writeTo() now invokes getSerializedSize(); >> >> + ByteArrayOutputStream outputStream = new >> > ByteArrayOutputStream(); > >> >> + CodedOutputStream codedOutput = >> >> CodedOutputStream.newInstance(outputStream); >> >> + >> >> + message.writeTo(codedOutput); >> >> + >> >> + codedOutput.flush(); >> >> + >> >> + TestPackedTypes message2 = TestPackedTypes.parseFrom( >> >> + outputStream.toByteArray()); >> >> + >> >> + TestUtil.assertPackedFieldsSet(message2); >> >> + } >> >> + >> >> public void testSerializeExtensionsLite() throws Exception { >> >> // TestAllTypes and TestAllExtensions should have compatible >> > wire > >> >> formats, >> >> // so if we serialize a TestAllExtensions then parse it as >> >> TestAllTypes >> >> Index: src/google/protobuf/compiler/java/java_primitive_field.cc >> >> =================================================================== >> >> --- src/google/protobuf/compiler/java/java_primitive_field.cc >> > (revision > >> >> 246) >> >> +++ src/google/protobuf/compiler/java/java_primitive_field.cc >> > (working > >> >> copy) >> >> @@ -384,8 +384,17 @@ >> >> void RepeatedPrimitiveFieldGenerator:: >> >> GenerateSerializationCode(io::Printer* printer) const { >> >> if (descriptor_->options().packed()) { >> >> + // writeTo() may be called without getSerializedSize() ever >> > having > >> >> been >> >> + // called, so we may need to compute the length of the packed >> > data. > >> >> Since >> >> + // the object is immutable, we know that the memoized size is >> > either > >> >> set to >> >> + // 0 (via object initialization) or else it has the correct >> > size from > >> >> a >> >> + // previous getSerializedSize() call. If the field is not >> > empty and > >> >> the size >> >> + // has not yet been computed, just call getSerializedSize() >> >> printer->Print(variables_, >> >> "if (get$capitalized_name$List().size() > 0) {\n" >> >> + " if ($name$MemoizedSerializedSize == 0) {\n" >> >> + " getSerializedSize();\n" >> >> + " }\n" >> >> " output.writeRawVarint32($tag$);\n" >> >> " output.writeRawVarint32($name$MemoizedSerializedSize);\n" >> >> "}\n" >> >> >> >> >> >> >> > >> > > > > > http://codereview.appspot.com/157169 >
Sign in to reply to this message.
Done. I kept the change to initialize the memoized size of packed fields to -1, just for consistency with the message's size. On 2009/11/25 20:51:52, kenton wrote: > You're right, we might as well always call it. > > On Wed, Nov 25, 2009 at 9:08 AM, <mailto:jasonh@google.com> wrote: > > > Done. But it occurs to me that this isn't all that much nicer a solution > > than just calling getSerializedSize() in writeTo(CodedOutputStream > > output), except that it only happens for messages with packed fields, > > rather than all messages. Should this compute only the serialized size > > of the packed field, so that it doesn't needlessly compute the size of > > all the unpacked fields? Or does this not really matter? I think all of > > the wrappers now call getSerializedSize() thanks to r240 (I was tearing > > my hair out trying to figure out why I could not get a new test which > > just did > > ByteArrayOutputStream output = new ByteArrayOutputStream(); > > message.writeTo(output); > > would not fail from the open source code, but would if I added the same > > test to our internal version, until I finally looked at the source). > > Anyway point being that in most cases getSerializedSize() will have > > already been called anyway so maybe it doesn't matter too much if we > > compute the size of the entire message rather than just the packed > > fields. > > > > > > On 2009/11/25 03:42:18, kenton wrote: > > > >> On Tue, Nov 24, 2009 at 7:40 PM, Kenton Varda > >> > > <mailto:kenton@google.com> wrote: > > > > > I guess it's technically the case that a packed field always has > >> > > non-zero > > > >> > size, but maybe we should initialize the cached sizes to -1 anyway > >> > > to make > > > >> > it more clear when they haven't been initialized? > >> > > > > > > That was a confusing sentence. Change "when they haven't been > >> > > initialized" > > > >> to "when getSerializedSize() hasn't been called". > >> > > > > > > > > >> > On Tue, Nov 24, 2009 at 7:04 PM, <mailto:jasonh@google.com> wrote: > >> > > >> >> Reviewers: kenton, > >> >> > >> >> Description: > >> >> Fix Issue 136: the memoized serialized size for packed fields may > >> > > not > > > >> >> be properly set. writeTo() may be invoked without a call to > >> >> getSerializedSize(), so the generated serialization methods would > >> >> write a length of 0 for non-empty packed fields. Since the object > >> >> is immutable, we know that the memoized size is 0 or set to the > >> >> correct size. In the generated code, check to see if the memoized > >> >> size is 0, and if so, call getSerializedSize() before actually > >> >> emitting the field. > >> >> > >> >> Tested: new unittest case in WireFormatTest.java now passes > >> >> > >> >> > >> >> > >> >> Please review this at http://codereview.appspot.com/157169 > >> >> > >> >> Affected files: > >> >> M java/src/test/java/com/google/protobuf/WireFormatTest.java > >> >> M src/google/protobuf/compiler/java/java_primitive_field.cc > >> >> > >> >> > >> >> Index: java/src/test/java/com/google/protobuf/WireFormatTest.java > >> >> =================================================================== > >> >> --- java/src/test/java/com/google/protobuf/WireFormatTest.java > >> > > (revision > > > >> >> 246) > >> >> +++ java/src/test/java/com/google/protobuf/WireFormatTest.java > >> > > (working > > > >> >> copy) > >> >> @@ -102,6 +102,28 @@ > >> >> assertEquals(rawBytes, rawBytes2); > >> >> } > >> >> > >> >> + public void testSerializationPackedWithoutGetSerializedSize() > >> >> + throws Exception { > >> >> + // Write directly to an OutputStream, without invoking > >> >> getSerializedSize() > >> >> + // This used to be a bug where the size of a packed field was > >> >> incorrect, > >> >> + // since getSerializedSize() was never invoked. > >> >> + TestPackedTypes message = TestUtil.getPackedSet(); > >> >> + > >> >> + // Directly construct a CodedOutputStream around the actual > >> >> OutputStream, > >> >> + // because writeTo() now invokes getSerializedSize(); > >> >> + ByteArrayOutputStream outputStream = new > >> > > ByteArrayOutputStream(); > > > >> >> + CodedOutputStream codedOutput = > >> >> CodedOutputStream.newInstance(outputStream); > >> >> + > >> >> + message.writeTo(codedOutput); > >> >> + > >> >> + codedOutput.flush(); > >> >> + > >> >> + TestPackedTypes message2 = TestPackedTypes.parseFrom( > >> >> + outputStream.toByteArray()); > >> >> + > >> >> + TestUtil.assertPackedFieldsSet(message2); > >> >> + } > >> >> + > >> >> public void testSerializeExtensionsLite() throws Exception { > >> >> // TestAllTypes and TestAllExtensions should have compatible > >> > > wire > > > >> >> formats, > >> >> // so if we serialize a TestAllExtensions then parse it as > >> >> TestAllTypes > >> >> Index: src/google/protobuf/compiler/java/java_primitive_field.cc > >> >> =================================================================== > >> >> --- src/google/protobuf/compiler/java/java_primitive_field.cc > >> > > (revision > > > >> >> 246) > >> >> +++ src/google/protobuf/compiler/java/java_primitive_field.cc > >> > > (working > > > >> >> copy) > >> >> @@ -384,8 +384,17 @@ > >> >> void RepeatedPrimitiveFieldGenerator:: > >> >> GenerateSerializationCode(io::Printer* printer) const { > >> >> if (descriptor_->options().packed()) { > >> >> + // writeTo() may be called without getSerializedSize() ever > >> > > having > > > >> >> been > >> >> + // called, so we may need to compute the length of the packed > >> > > data. > > > >> >> Since > >> >> + // the object is immutable, we know that the memoized size is > >> > > either > > > >> >> set to > >> >> + // 0 (via object initialization) or else it has the correct > >> > > size from > > > >> >> a > >> >> + // previous getSerializedSize() call. If the field is not > >> > > empty and > > > >> >> the size > >> >> + // has not yet been computed, just call getSerializedSize() > >> >> printer->Print(variables_, > >> >> "if (get$capitalized_name$List().size() > 0) {\n" > >> >> + " if ($name$MemoizedSerializedSize == 0) {\n" > >> >> + " getSerializedSize();\n" > >> >> + " }\n" > >> >> " output.writeRawVarint32($tag$);\n" > >> >> " output.writeRawVarint32($name$MemoizedSerializedSize);\n" > >> >> "}\n" > >> >> > >> >> > >> >> > >> > > >> > > > > > > > > > > http://codereview.appspot.com/157169 > > >
Sign in to reply to this message.
LGTM, just a minor nitpick: http://codereview.appspot.com/157169/diff/2007/1012 File src/google/protobuf/compiler/java/java_message.cc (right): http://codereview.appspot.com/157169/diff/2007/1012#newcode404 src/google/protobuf/compiler/java/java_message.cc:404: "getSerializedSize();"); Missing \n.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
