From 8b2de9a0f58feda944961547c43707eec993463b Mon Sep 17 00:00:00 2001 From: Jake Rich Date: Fri, 31 May 2024 12:55:03 -0400 Subject: [PATCH 1/5] Skip serialization of optional fields set to their default value --- .../CodeGenerator/FieldSerializer.cs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/CodeGenerator/CodeGenerator/FieldSerializer.cs b/CodeGenerator/CodeGenerator/FieldSerializer.cs index 8682cd3..dbaf307 100644 --- a/CodeGenerator/CodeGenerator/FieldSerializer.cs +++ b/CodeGenerator/CodeGenerator/FieldSerializer.cs @@ -335,6 +335,16 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP } else if (f.Rule == FieldRule.Optional) { + // Don't serialize fields if they are their default value + // Should be safe to use on every single type, as long as it doesn't apply during delta comparison + bool skippedDefaultValue = false; + bool canOptional = f.OptionDefault == null; + if (canOptional && !hasPrevious) + { + skippedDefaultValue = true; + cw.IfBracket( "instance." + f.CsName + " != default" ); + } + if (f.ProtoType is ProtoMessage || f.ProtoType.ProtoName == ProtoBuiltin.String || f.ProtoType.ProtoName == ProtoBuiltin.Bytes) @@ -357,6 +367,9 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( hasPrevious && canDelta ) cw.EndBracket(); + + if (skippedDefaultValue) + cw.EndBracket(); return; } if (f.ProtoType is ProtoEnum) @@ -367,6 +380,8 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) ); if (f.OptionDefault != null) cw.EndBracket(); + if (skippedDefaultValue) + cw.EndBracket(); return; } @@ -376,7 +391,9 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP KeyWriter("stream", f.ID, f.ProtoType.WireType, cw); cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) ); - if ( hasPrevious && canDelta ) + if ( hasPrevious && canDelta ) + cw.EndBracket(); + if (skippedDefaultValue) cw.EndBracket(); return; } From 2c5f5f373fa5d4a24ba6bae90701fb566b895580 Mon Sep 17 00:00:00 2001 From: Jake Rich Date: Fri, 31 May 2024 13:19:22 -0400 Subject: [PATCH 2/5] Also exclude serializing default value of required fields --- .../CodeGenerator/FieldSerializer.cs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/CodeGenerator/CodeGenerator/FieldSerializer.cs b/CodeGenerator/CodeGenerator/FieldSerializer.cs index dbaf307..c1da65a 100644 --- a/CodeGenerator/CodeGenerator/FieldSerializer.cs +++ b/CodeGenerator/CodeGenerator/FieldSerializer.cs @@ -285,6 +285,10 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( f.ProtoType.ProtoName == ProtoBuiltin.UInt64 ) canDelta = true; if ( f.ProtoType.ProtoName == ProtoBuiltin.Double ) canDelta = true; + // Don't serialize fields if they are their default value + // Should be safe to use on every single type, as long as it doesn't apply during delta comparison + bool canOptional = f.OptionDefault == null && !hasPrevious; + if (f.Rule == FieldRule.Repeated) { if (f.OptionPacked == true) @@ -335,13 +339,8 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP } else if (f.Rule == FieldRule.Optional) { - // Don't serialize fields if they are their default value - // Should be safe to use on every single type, as long as it doesn't apply during delta comparison - bool skippedDefaultValue = false; - bool canOptional = f.OptionDefault == null; - if (canOptional && !hasPrevious) + if (canOptional) { - skippedDefaultValue = true; cw.IfBracket( "instance." + f.CsName + " != default" ); } @@ -368,7 +367,7 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( hasPrevious && canDelta ) cw.EndBracket(); - if (skippedDefaultValue) + if (canOptional) cw.EndBracket(); return; } @@ -380,7 +379,7 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) ); if (f.OptionDefault != null) cw.EndBracket(); - if (skippedDefaultValue) + if (canOptional) cw.EndBracket(); return; } @@ -393,12 +392,17 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( hasPrevious && canDelta ) cw.EndBracket(); - if (skippedDefaultValue) + if (canOptional) cw.EndBracket(); return; } else if (f.Rule == FieldRule.Required) { + if (canOptional) + { + cw.IfBracket( "instance." + f.CsName + " != default" ); + } + if ( hasPrevious && canDelta ) cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName ); @@ -419,6 +423,9 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( hasPrevious && canDelta ) cw.EndBracket(); + if ( canOptional ) + cw.EndBracket(); + return; } throw new NotImplementedException("Unknown rule: " + f.Rule); From 1f15044064c9f925798a20c9a26b27d7ee79ee40 Mon Sep 17 00:00:00 2001 From: Jake Rich Date: Tue, 18 Jun 2024 18:21:42 -0400 Subject: [PATCH 3/5] Add support for all default values, fix optional field checks applying to DeltaSerialize() --- .../CodeGenerator/FieldSerializer.cs | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/CodeGenerator/CodeGenerator/FieldSerializer.cs b/CodeGenerator/CodeGenerator/FieldSerializer.cs index c1da65a..cbb1ac1 100644 --- a/CodeGenerator/CodeGenerator/FieldSerializer.cs +++ b/CodeGenerator/CodeGenerator/FieldSerializer.cs @@ -271,6 +271,23 @@ static void BytesWriter(Field f, string stream, CodeWriter cw) cw.WriteLine(stream + ".Write(msField.GetBuffer(), 0, (int)length" + f.ID + ");"); } + private static void WriteOptionalFieldCheck( CodeWriter cw, Field f ) + { + // Support fields with custom default values + string defaultString = f.OptionDefault ?? "default"; + + // For some reason Ray isn't comparible so just check it's fields + if (f.ProtoTypeName == "Ray") + { + cw.IfBracket( $"instance.{f.CsName}.origin != default && instance.{f.CsName}.direction != default" ); + } + else + { + // Normally compare to default + cw.IfBracket( $"instance.{f.CsName} != {defaultString}" ); + } + } + /// /// Generates code for writing one field /// @@ -285,9 +302,8 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( f.ProtoType.ProtoName == ProtoBuiltin.UInt64 ) canDelta = true; if ( f.ProtoType.ProtoName == ProtoBuiltin.Double ) canDelta = true; - // Don't serialize fields if they are their default value - // Should be safe to use on every single type, as long as it doesn't apply during delta comparison - bool canOptional = f.OptionDefault == null && !hasPrevious; + // Don't change behavior of SerializeDelta() calls + bool canOptional = !hasPrevious; if (f.Rule == FieldRule.Repeated) { @@ -398,18 +414,16 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP } else if (f.Rule == FieldRule.Required) { - if (canOptional) - { - cw.IfBracket( "instance." + f.CsName + " != default" ); - } - - if ( hasPrevious && canDelta ) + if ( hasPrevious && canDelta ) cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName ); if (f.ProtoType is ProtoMessage && f.ProtoType.OptionType != "struct" || f.ProtoType.ProtoName == ProtoBuiltin.String || f.ProtoType.ProtoName == ProtoBuiltin.Bytes) { + // Don't do extra optional checks if it's trying to throw an error + canOptional = false; + if (f.ProtoType.ProtoName == ProtoBuiltin.Bytes && f.OptionPooled) cw.WriteLine("if (instance." + f.CsName + ".Array == null)"); else @@ -417,6 +431,12 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP cw.WriteIndent("throw new ArgumentNullException(\"" + f.CsName + "\", \"Required by proto specification.\");"); } + + // Skip default values of fields + if ( canOptional) + { + WriteOptionalFieldCheck( cw, f ); + } KeyWriter("stream", f.ID, f.ProtoType.WireType, cw); cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) ); From ffc464923682c3f8ff5639706209676c5656cc28 Mon Sep 17 00:00:00 2001 From: Jake Rich Date: Tue, 18 Jun 2024 18:37:40 -0400 Subject: [PATCH 4/5] Make sure existing null checks aren't applied on top of new default value checks --- .../CodeGenerator/FieldSerializer.cs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/CodeGenerator/CodeGenerator/FieldSerializer.cs b/CodeGenerator/CodeGenerator/FieldSerializer.cs index cbb1ac1..b451ccc 100644 --- a/CodeGenerator/CodeGenerator/FieldSerializer.cs +++ b/CodeGenerator/CodeGenerator/FieldSerializer.cs @@ -355,23 +355,26 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP } else if (f.Rule == FieldRule.Optional) { - if (canOptional) - { - cw.IfBracket( "instance." + f.CsName + " != default" ); - } - if (f.ProtoType is ProtoMessage || f.ProtoType.ProtoName == ProtoBuiltin.String || f.ProtoType.ProtoName == ProtoBuiltin.Bytes) { if (f.ProtoType.Nullable) //Struct always exist, not optional { + // Manual null checks above = don't do default value checks + canOptional = false; + if (f.ProtoType.ProtoName == ProtoBuiltin.Bytes && f.OptionPooled) cw.IfBracket("instance." + f.CsName + ".Array != null"); else cw.IfBracket("instance." + f.CsName + " != null"); } + if (canOptional) + { + WriteOptionalFieldCheck( cw, f ); + } + if ( hasPrevious && canDelta ) cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName ); @@ -391,6 +394,10 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP { if (f.OptionDefault != null) cw.IfBracket("instance." + f.CsName + " != " + f.ProtoType.CsType + "." + f.OptionDefault); + if ( canOptional) + { + WriteOptionalFieldCheck( cw, f ); + } KeyWriter("stream", f.ID, f.ProtoType.WireType, cw); cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) ); if (f.OptionDefault != null) @@ -403,6 +410,11 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( hasPrevious && canDelta ) cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName ); + if ( canOptional) + { + WriteOptionalFieldCheck( cw, f ); + } + KeyWriter("stream", f.ID, f.ProtoType.WireType, cw); cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) ); From 5a5747fe78f9b65266ddd008e1e0ea8f3c2a51dc Mon Sep 17 00:00:00 2001 From: Jake Rich Date: Fri, 13 Dec 2024 08:26:58 -0500 Subject: [PATCH 5/5] Don't apply to enums --- CodeGenerator/CodeGenerator/FieldSerializer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CodeGenerator/CodeGenerator/FieldSerializer.cs b/CodeGenerator/CodeGenerator/FieldSerializer.cs index b451ccc..25b0331 100644 --- a/CodeGenerator/CodeGenerator/FieldSerializer.cs +++ b/CodeGenerator/CodeGenerator/FieldSerializer.cs @@ -303,7 +303,8 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP if ( f.ProtoType.ProtoName == ProtoBuiltin.Double ) canDelta = true; // Don't change behavior of SerializeDelta() calls - bool canOptional = !hasPrevious; + // Don't skip enums when default values because their default may be awkward + bool canOptional = !hasPrevious && !(f.ProtoType is ProtoEnum); if (f.Rule == FieldRule.Repeated) {