Skip to content

Commit

Permalink
Use ConcurrentDictionary in JsonFormatter (#14272)
Browse files Browse the repository at this point in the history
I have a theory that the lock in `JsonFormatter` periodically causes this VS debugging error for Protobuf types:

![image](https://github.com/protocolbuffers/protobuf/assets/303201/64097805-67ab-43fe-a4a0-77ec4590844d)

Hopefully `ConcurrentDictionary` resolves this issue. Even if it doesn't, this change still resolves the TODO so this is a good improvement.

Closes #14272

COPYBARA_INTEGRATE_REVIEW=#14272 from JamesNK:jamesnk/concurrentdictionary 6ec3ebc
PiperOrigin-RevId: 570439102
  • Loading branch information
JamesNK authored and Copybara-Service committed Oct 3, 2023
1 parent 95d5723 commit 68fce2e
Showing 1 changed file with 9 additions and 19 deletions.
28 changes: 9 additions & 19 deletions csharp/src/Google.Protobuf/JsonFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@

using System;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Text;
using Google.Protobuf.Reflection;
using Google.Protobuf.WellKnownTypes;
using System.IO;
using System.Linq;
using System.Collections.Generic;
using System.Reflection;
using System.Diagnostics.CodeAnalysis;
using System.Text;
using Google.Protobuf.Reflection;
using Google.Protobuf.WellKnownTypes;

namespace Google.Protobuf
{
Expand Down Expand Up @@ -962,25 +963,14 @@ public Settings(bool formatDefaultValues, TypeRegistry typeRegistry) : this(form
// The need for this is unfortunate, as is its unbounded size, but realistically it shouldn't cause issues.
private static class OriginalEnumValueHelper
{
// TODO: In the future we might want to use ConcurrentDictionary, at the point where all
// the platforms we target have it.
private static readonly Dictionary<System.Type, Dictionary<object, string>> dictionaries
= new Dictionary<System.Type, Dictionary<object, string>>();
private static readonly ConcurrentDictionary<System.Type, Dictionary<object, string>> dictionaries
= new ConcurrentDictionary<System.Type, Dictionary<object, string>>();

[UnconditionalSuppressMessage("Trimming", "IL2072",
Justification = "The field for the value must still be present. It will be returned by reflection, will be in this collection, and its name can be resolved.")]
internal static string GetOriginalName(object value)
{
var enumType = value.GetType();
Dictionary<object, string> nameMapping;
lock (dictionaries)
{
if (!dictionaries.TryGetValue(enumType, out nameMapping))
{
nameMapping = GetNameMapping(enumType);
dictionaries[enumType] = nameMapping;
}
}
Dictionary<object, string> nameMapping = dictionaries.GetOrAdd(value.GetType(), static t => GetNameMapping(t));

// If this returns false, originalName will be null, which is what we want.
nameMapping.TryGetValue(value, out string originalName);
Expand Down

0 comments on commit 68fce2e

Please sign in to comment.