[c#] Moved all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load. #7722

ManuelKugelmann · 2020-07-16T12:48:25Z

Moves all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load.

googlebot · 2020-07-16T12:48:36Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

ManuelKugelmann · 2020-07-16T12:55:27Z

@googlebot I signed it!

googlebot · 2020-07-17T06:55:57Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

jtattermusch · 2020-07-17T07:00:47Z

src/google/protobuf/compiler/csharp/csharp_message_field.cc

@@ -104,7 +104,7 @@ void MessageFieldGenerator::GenerateMergingCode(io::Printer* printer) {
    variables_,
    "if (other.$has_property_check$) {\n"
    "  if ($has_not_property_check$) {\n"
-    "    $property_name$ = new $type_name$();\n"
+    "    $property_name$ = $type_name$.Parser.CreateTemplate();\n"


after changing the codegen, you'd also need to regenerate the code in this repository.

= do a build ?

Build and then run https://github.com/protocolbuffers/protobuf/blob/master/csharp/generate_protos.sh.

The updates to the generated files need to be commited (preferrably as a separate commit)

jtattermusch · 2020-07-17T07:01:18Z

CC @JamesNK @jskeet

JamesNK · 2020-07-17T09:02:14Z

csharp/src/Google.Protobuf/MessageParser.cs

@@ -60,7 +60,7 @@ internal MessageParser(Func<IMessage> factory, bool discardUnknownFields, Extens
        /// Creates a template instance ready for population.
        /// </summary>
        /// <returns>An empty message.</returns>
-        internal IMessage CreateTemplate()
+        public IMessage CreateTemplate()


JamesNK · 2020-07-17T09:02:38Z

src/google/protobuf/compiler/csharp/csharp_message_field.cc

@@ -104,7 +104,7 @@ void MessageFieldGenerator::GenerateMergingCode(io::Printer* printer) {
    variables_,
    "if (other.$has_property_check$) {\n"
    "  if ($has_not_property_check$) {\n"
-    "    $property_name$ = new $type_name$();\n"
+    "    $property_name$ = $type_name$.Parser.CreateTemplate();\n"


What is the overhead of using Parser.CreateTemplate?

jtattermusch · 2020-07-20T08:58:13Z

Is there a good way of checking that we haven't missed any occurrences of new XYZMessage() in the code?

jtattermusch · 2020-07-20T12:14:26Z

One potential concern is that if this gets merged, the newly generated code won't be compatible with the older runtimes anymore (there is no public CreateTemplate() method).
While that is fine and understandable limitation, it would still be better if this change is bundled with other changes that also require runtime upgrade once code is regenerated, to amortize the cost of potentially disrupting user's workflow over multiple features. This feature in isolation is quite a niche case so I'm not sure the benefits outweigh the costs.

acozzette · 2021-10-13T19:01:15Z

@ManuelKugelmann I am going to close this since there has been no activity in a long time, but please feel free to comment here if you would like to pick this up again.

ahmednfwela · 2024-12-23T00:10:33Z

@jtattermusch I have issued #19776 to only change the visibility of the CreateTemplate() method.

replace new by Parser.CreateTemplate

f46804c

Moves all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load.

googlebot added the cla: no label Jul 16, 2020

ManuelKugelmann changed the title ~~fix issue #6401 - replace new by Parser.CreateTemplate~~ [c#] fix issue #6401 - replace new by Parser.CreateTemplate Jul 16, 2020

ManuelKugelmann changed the title ~~[c#] fix issue #6401 - replace new by Parser.CreateTemplate~~ [c#] Moved all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load. Jul 16, 2020

jtattermusch added c# kokoro:run labels Jul 17, 2020

protobuf-kokoro removed the kokoro:run label Jul 17, 2020

googlebot added cla: yes and removed cla: no labels Jul 17, 2020

jtattermusch reviewed Jul 17, 2020

View reviewed changes

jtattermusch self-assigned this Jul 17, 2020

JamesNK reviewed Jul 17, 2020

View reviewed changes

jtattermusch added the release notes: yes label Jul 20, 2020

acozzette closed this Oct 13, 2021

ahmednfwela mentioned this pull request Dec 23, 2024

feat(csharp/Reflection): change MessageParser.CreateTemplate to be public #19776

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[c#] Moved all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load. #7722

[c#] Moved all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load. #7722

ManuelKugelmann commented Jul 16, 2020 •

edited

Loading

googlebot commented Jul 16, 2020

ManuelKugelmann commented Jul 16, 2020 •

edited

Loading

googlebot commented Jul 17, 2020

jtattermusch Jul 17, 2020

ManuelKugelmann Jul 20, 2020 •

edited

Loading

jtattermusch Jul 20, 2020

jtattermusch commented Jul 17, 2020 •

edited

Loading

JamesNK Jul 17, 2020

JamesNK Jul 17, 2020

jtattermusch commented Jul 20, 2020

jtattermusch commented Jul 20, 2020

acozzette commented Oct 13, 2021

ahmednfwela commented Dec 23, 2024

[c#] Moved all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load. #7722

[c#] Moved all message allocations to CreateTemplate(), making it easier to e.g. use pools to prevent GC load. #7722

Conversation

ManuelKugelmann commented Jul 16, 2020 • edited Loading

googlebot commented Jul 16, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

ManuelKugelmann commented Jul 16, 2020 • edited Loading

googlebot commented Jul 17, 2020

jtattermusch Jul 17, 2020

Choose a reason for hiding this comment

ManuelKugelmann Jul 20, 2020 • edited Loading

Choose a reason for hiding this comment

jtattermusch Jul 20, 2020

Choose a reason for hiding this comment

jtattermusch commented Jul 17, 2020 • edited Loading

JamesNK Jul 17, 2020

Choose a reason for hiding this comment

JamesNK Jul 17, 2020

Choose a reason for hiding this comment

jtattermusch commented Jul 20, 2020

jtattermusch commented Jul 20, 2020

acozzette commented Oct 13, 2021

ahmednfwela commented Dec 23, 2024

ManuelKugelmann commented Jul 16, 2020 •

edited

Loading

ManuelKugelmann commented Jul 16, 2020 •

edited

Loading

ManuelKugelmann Jul 20, 2020 •

edited

Loading

jtattermusch commented Jul 17, 2020 •

edited

Loading