Skip to content

Commit

Permalink
C#: Avoid string concatenation when looking up enum values by name.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 616745400
  • Loading branch information
protobuf-github-bot authored and Copybara-Service committed Mar 18, 2024
1 parent 71eed03 commit e6684ac
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 35 deletions.
106 changes: 76 additions & 30 deletions csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ internal sealed class DescriptorPool
private readonly IDictionary<ObjectIntPair<IDescriptor>, EnumValueDescriptor> enumValuesByNumber =
new Dictionary<ObjectIntPair<IDescriptor>, EnumValueDescriptor>();

private readonly IDictionary<EnumValueByNameDescriptorKey, EnumValueDescriptor> enumValuesByName =
new Dictionary<EnumValueByNameDescriptorKey, EnumValueDescriptor>();

private readonly HashSet<FileDescriptor> dependencies = new HashSet<FileDescriptor>();

internal DescriptorPool(IEnumerable<FileDescriptor> dependencyFiles)
Expand Down Expand Up @@ -129,29 +132,20 @@ internal void AddSymbol(IDescriptor descriptor)

if (descriptorsByName.TryGetValue(fullName, out IDescriptor old))
{
int dotPos = fullName.LastIndexOf('.');
string message;
if (descriptor.File == old.File)
{
if (dotPos == -1)
{
message = "\"" + fullName + "\" is already defined.";
}
else
{
message = "\"" + fullName.Substring(dotPos + 1) + "\" is already defined in \"" +
fullName.Substring(0, dotPos) + "\".";
}
}
else
{
message = "\"" + fullName + "\" is already defined in file \"" + old.File.Name + "\".";
}
throw new DescriptorValidationException(descriptor, message);
throw new DescriptorValidationException(descriptor,
GetDescriptorAlreadyAddedExceptionMessage(descriptor, fullName, old));
}
descriptorsByName[fullName] = descriptor;
}

private static string GetDescriptorAlreadyAddedExceptionMessage(IDescriptor descriptor, string fullName, IDescriptor old)
{
int dotPos = fullName.LastIndexOf('.');
return descriptor.File != old.File ? $"\"{fullName}\" is already defined in file \"{old.File.Name}\"."
: dotPos == -1 ? $"{fullName} is already defined."
: $"\"{fullName.Substring(dotPos + 1)}\" is already defined in \"{fullName.Substring(0, dotPos)}\".";
}

/// <summary>
/// Verifies that the descriptor's name is valid (i.e. it contains
/// only letters, digits and underscores, and does not start with a digit).
Expand All @@ -167,11 +161,14 @@ private static void ValidateSymbolName(IDescriptor descriptor)
// Symbol name must start with a letter or underscore, and it can contain letters,
// numbers and underscores.
string name = descriptor.Name;
if (!IsAsciiLetter(name[0]) && name[0] != '_') {
if (!IsAsciiLetter(name[0]) && name[0] != '_')
{
ThrowInvalidSymbolNameException(descriptor);
}
for (int i = 1; i < name.Length; i++) {
if (!IsAsciiLetter(name[i]) && !IsAsciiDigit(name[i]) && name[i] != '_') {
for (int i = 1; i < name.Length; i++)
{
if (!IsAsciiLetter(name[i]) && !IsAsciiDigit(name[i]) && name[i] != '_')
{
ThrowInvalidSymbolNameException(descriptor);
}
}
Expand Down Expand Up @@ -199,6 +196,12 @@ internal EnumValueDescriptor FindEnumValueByNumber(EnumDescriptor enumDescriptor
return ret;
}

internal EnumValueDescriptor FindEnumValueByName(EnumDescriptor enumDescriptor, string name)
{
enumValuesByName.TryGetValue(new EnumValueByNameDescriptorKey(enumDescriptor, name), out EnumValueDescriptor ret);
return ret;
}

/// <summary>
/// Adds a field to the fieldsByNumber table.
/// </summary>
Expand All @@ -219,17 +222,28 @@ internal void AddFieldByNumber(FieldDescriptor field)
}

/// <summary>
/// Adds an enum value to the enumValuesByNumber table. If an enum value
/// with the same type and number already exists, this method does nothing.
/// (This is allowed; the first value defined with the number takes precedence.)
/// Adds an enum value to the enumValuesByNumber and enumValuesByName tables. If an enum value
/// with the same type and number already exists, this method does nothing to enumValuesByNumber.
/// (This is allowed; the first value defined with the number takes precedence.) If an enum
/// value with the same name already exists, this method throws DescriptorValidationException.
/// (It is expected that this method is called after AddSymbol, which would already have thrown
/// an exception in this failure case.)
/// </summary>
internal void AddEnumValueByNumber(EnumValueDescriptor enumValue)
internal void AddEnumValue(EnumValueDescriptor enumValue)
{
ObjectIntPair<IDescriptor> key = new ObjectIntPair<IDescriptor>(enumValue.EnumDescriptor, enumValue.Number);
if (!enumValuesByNumber.ContainsKey(key))
ObjectIntPair<IDescriptor> numberKey = new ObjectIntPair<IDescriptor>(enumValue.EnumDescriptor, enumValue.Number);
if (!enumValuesByNumber.ContainsKey(numberKey))
{
enumValuesByNumber[key] = enumValue;
enumValuesByNumber[numberKey] = enumValue;
}

EnumValueByNameDescriptorKey nameKey = new EnumValueByNameDescriptorKey(enumValue.EnumDescriptor, enumValue.Name);
if (enumValuesByName.TryGetValue(nameKey, out EnumValueDescriptor old))
{
throw new DescriptorValidationException(enumValue,
GetDescriptorAlreadyAddedExceptionMessage(enumValue, enumValue.FullName, old));
}
enumValuesByName[nameKey] = enumValue;
}

/// <summary>
Expand Down Expand Up @@ -308,5 +322,37 @@ internal IDescriptor LookupSymbol(string name, IDescriptor relativeTo)
return result;
}
}
}

/// <summary>
/// Struct used to hold the keys for the enumValuesByName table.
/// </summary>
private struct EnumValueByNameDescriptorKey : IEquatable<EnumValueByNameDescriptorKey>
{
private readonly string name;
private readonly IDescriptor descriptor;

internal EnumValueByNameDescriptorKey(EnumDescriptor descriptor, string valueName)
{
this.descriptor = descriptor;
this.name = valueName;
}

public bool Equals(EnumValueByNameDescriptorKey other) =>
descriptor == other.descriptor
&& name == other.name;

public override bool Equals(object obj) =>
obj is EnumValueByNameDescriptorKey pair && Equals(pair);

public override int GetHashCode()
{
unchecked
{
var hashCode = descriptor.GetHashCode();
hashCode = (hashCode * 397) ^ (name != null ? name.GetHashCode() : 0);
return hashCode;
}
}
}
}
}
6 changes: 2 additions & 4 deletions csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ public EnumValueDescriptor FindValueByNumber(int number)
/// </summary>
/// <param name="name">The unqualified name of the value (e.g. "FOO").</param>
/// <returns>The value's descriptor, or null if not found.</returns>
public EnumValueDescriptor FindValueByName(string name)
{
return File.DescriptorPool.FindSymbol<EnumValueDescriptor>(FullName + "." + name);
}
public EnumValueDescriptor FindValueByName(string name) =>
File.DescriptorPool.FindEnumValueByName(this, name);

/// <summary>
/// The (possibly empty) set of custom options for this enum.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public sealed class EnumValueDescriptor : DescriptorBase
Proto = proto;
EnumDescriptor = parent;
file.DescriptorPool.AddSymbol(this);
file.DescriptorPool.AddEnumValueByNumber(this);
file.DescriptorPool.AddEnumValue(this);
}

internal EnumValueDescriptorProto Proto { get; }
Expand Down

0 comments on commit e6684ac

Please sign in to comment.