From 43f4f2f509bec5b792409bc06fa3461129c4c611 Mon Sep 17 00:00:00 2001 From: dsavvinov Date: Thu, 11 Aug 2016 15:20:48 +0300 Subject: [PATCH] Protobuf: fixed a bug in estimating size of field of type = packed array. Now size of arrays properly adds to size of field, but not to size of array itself --- .../kotlin/src/kotlin_field_generator.cc | 141 +++++++++++------- .../kotlin/src/kotlin_field_generator.h | 5 + 2 files changed, 92 insertions(+), 54 deletions(-) diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc index c1e3b4fcbe7..508a4217de9 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc @@ -176,7 +176,6 @@ void FieldGenerator::generateSerializationForPacked(io::Printer *printer, bool i } void FieldGenerator::generateSerializationForEnums(io::Printer * printer, bool isRead, bool noTag, bool isField) const { - map vars; vars["converter"] = getEnumFromIntConverter(); vars["fieldName"] = simpleName; @@ -372,53 +371,97 @@ string FieldGenerator::getKotlinFunctionSuffix() const { return name_resolving::protobufTypeToKotlinFunctionSuffix(descriptor->type()); } -void FieldGenerator::generateSizeEstimationCode(io::Printer *printer, string varName, bool noTag, bool isField) const { + +void FieldGenerator::generateSizeForPacked(io::Printer * printer, string varName, bool noTag, bool isField) const { map vars; vars["varName"] = varName; vars["fieldName"] = simpleName; vars["fieldNumber"] = std::to_string(getFieldNumber()); + printer->Print(vars, "if ($fieldName$.size != 0) {\n"); + printer->Indent(); + + // We will need total byte size of array, because that size is itself a part of the message and + // adds to total message size. + // For the sake of hygiene, temporary variables are created in anonymous scope + printer->Print("do {\n"); + printer->Indent(); + + // Create a temporary variable that will collect array byte size + printer->Print("var arraySize = 0\n"); + + // Add tag size, if necessary + if (!noTag) { + printer->Print(vars, "arraySize += WireFormat.getTagSize($fieldNumber$, WireType.LENGTH_DELIMITED)\n"); + } + + // iterate over all elements of array + printer->Print("var i = 0\n"); + printer->Print(vars, "while (i < $fieldName$.size) {\n"); + printer->Indent(); + + // hack: reuse generateSizeEstimationCode in the same manner as in generateSerializationCode + FieldGenerator singleFieldGen = FieldGenerator(descriptor, enclosingClass, nameResolver); + singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL; + singleFieldGen.simpleName = simpleName + "[i]"; + singleFieldGen.generateSizeEstimationCode(printer, "arraySize", /* noTag = */ true, /* isField = */ false); + + printer->Print(vars, "i += 1\n"); + + printer->Outdent(); // for-loop + printer->Print("} \n"); + + // now add size of array to total message size: + printer->Print(vars, + "$varName$ += arraySize\n"); // actual array size + + // add size of size annotation: + if (!noTag) { + printer->Print(vars, "$varName$ += WireFormat.getInt32SizeNoTag(arraySize)\n"); + } + + printer->Outdent(); // anonymous scope + printer->Print("} while(false)\n"); + + printer->Outdent(); // if-clause + printer->Print("}\n"); +} + +void FieldGenerator::generateSizeForEnums(io::Printer *printer, string varName, bool noTag, bool isField) const { + map vars; + vars["varName"] = varName; + vars["fieldName"] = simpleName; + vars["fieldNumber"] = std::to_string(getFieldNumber()); + printer->Print(vars, "$varName$ += WireFormat.getEnumSize($fieldNumber$, $fieldName$.ord)\n"); +} + +void FieldGenerator::generateSizeForMessages(io::Printer * printer, string varName, bool noTag, bool isField) const { + map vars; + vars["varName"] = varName; + vars["fieldName"] = simpleName; + vars["maybeNoTag"] = noTag ? "NoTag" : ""; + vars["maybeFieldNumber"] = noTag ? "" : std::to_string(getFieldNumber()); + printer->Print(vars, "$varName$ += $fieldName$.getSize$maybeNoTag$($maybeFieldNumber$)" + "\n"); +} + +void FieldGenerator::generateSizeForPrimitives(io::Printer * printer, string varName, bool noTag, bool isField) const { + map vars; + vars["varName"] = varName; + vars["fieldName"] = simpleName; + vars["kotlinSuffix"] = getKotlinFunctionSuffix(); + vars["noTag"] = noTag ? "NoTag" : ""; + vars["fn"] = noTag ? "" : std::to_string(getFieldNumber()) + ", "; + printer->Print(vars, "$varName$ += WireFormat.get$kotlinSuffix$Size$noTag$($fn$$fieldName$)\n"); +} + +void FieldGenerator::generateSizeEstimationCode(io::Printer *printer, string varName, bool noTag, bool isField) const { + map vars; + vars["fieldName"] = simpleName; vars["initValue"] = getInitValue(); // First of all, generate code for repeated fields if (getProtoLabel() == FieldDescriptor::LABEL_REPEATED) { - printer->Print(vars, "if ($fieldName$.size != 0) {\n"); - printer->Indent(); - - // We will need total byte size of array, because that size is itself a part of the message and - // adds to total message size. - // For the sake of hygiene, temporary variables are created in anonymous scope - printer->Print("do {\n"); - printer->Indent(); - - // Create a temporary variable that will collect array byte size - printer->Print("var arraySize = 0\n"); - - // iterate over all elements of array - printer->Print("var i = 0\n"); - printer->Print(vars, "while (i < $fieldName$.size) {\n"); - printer->Indent(); - - // hack: reuse generateSizeEstimationCode in the same manner as in generateSerializationCode - FieldGenerator singleFieldGen = FieldGenerator(descriptor, enclosingClass, nameResolver); - singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL; - singleFieldGen.simpleName = simpleName + "[i]"; - singleFieldGen.generateSizeEstimationCode(printer, "arraySize", noTag, /* isField = */ false); - - printer->Print(vars, "i += 1\n"); - - printer->Outdent(); // for-loop - printer->Print("} \n"); - - // now add size of array to total message size: - printer->Print(vars, - "$varName$ += arraySize"); // actual array size - printer->Print("\n"); - printer->Outdent(); // anonymous scope - printer->Print("} while(false)\n"); - - printer->Outdent(); // if-clause - printer->Print("}\n"); - + generateSizeForPacked(printer, varName, noTag, isField); return; } @@ -430,26 +473,16 @@ void FieldGenerator::generateSizeEstimationCode(io::Printer *printer, string var } // Then, call getSize recursively for nested messages - // TODO: currently suboptimal repeatative calls getSize() are being made. We can optimize it later via caching calls to getSize() if (getProtoType() == FieldDescriptor::TYPE_MESSAGE) { - vars["maybeNoTag"] = noTag ? "NoTag" : ""; - vars["maybeFieldNumber"] = noTag ? "" : std::to_string(getFieldNumber()); - printer->Print(vars, "$varName$ += $fieldName$.getSize$maybeNoTag$($maybeFieldNumber$)" - "\n"); + generateSizeForMessages(printer, varName, noTag, isField); } - - // Next, process enums as they should be casted to ints manually else if (getProtoType() == FieldDescriptor::TYPE_ENUM) { - printer->Print(vars, "$varName$ += WireFormat.getEnumSize($fieldNumber$, $fieldName$.ord)\n"); + generateSizeForEnums(printer, varName, noTag, isField); } - // Finally, get size of all primitive types trivially via call to WireFormat in runtime else { - vars["kotlinSuffix"] = getKotlinFunctionSuffix(); - vars["noTag"] = noTag ? "NoTag" : ""; - vars["fn"] = noTag ? "" : std::to_string(getFieldNumber()) + ", "; - printer->Print(vars, "$varName$ += WireFormat.get$kotlinSuffix$Size$noTag$($fn$$fieldName$)\n"); + generateSizeForPrimitives(printer, varName, noTag, isField); } if (isField) { @@ -462,6 +495,7 @@ FieldDescriptor::Label FieldGenerator::getProtoLabel() const { return protoLabel; } + FieldDescriptor::Type FieldGenerator::getProtoType() const { return descriptor->type(); } @@ -470,7 +504,6 @@ int FieldGenerator::getFieldNumber() const { return descriptor->number(); } - string FieldGenerator::getSimpleType() const { if (getProtoLabel() == FieldDescriptor::LABEL_REPEATED) { return "MutableList <" + getUnderlyingTypeGenerator().getSimpleType() + ">"; diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.h b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.h index 0d03e18b6c2..799239ecef2 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.h +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.h @@ -31,6 +31,11 @@ private: void generateSerializationForEnums (io::Printer * printer, bool isRead, bool noTag, bool isField) const; void generateSerializationForMessages (io::Printer * printer, bool isRead, bool noTag, bool isField) const; void generateSerializationForPrimitives (io::Printer * printer, bool isRead, bool noTag, bool isField) const; + + void generateSizeForPacked (io::Printer * printer, string varName, bool noTag, bool isField) const; + void generateSizeForEnums (io::Printer * printer, string varName, bool noTag, bool isField) const; + void generateSizeForMessages (io::Printer * printer, string varName, bool noTag, bool isField) const; + void generateSizeForPrimitives (io::Printer * printer, string varName, bool noTag, bool isField) const; public: ClassGenerator const * enclosingClass; // class, in which that field is defined NameResolver * nameResolver;