Skip to content

CSHARP-2450 (second attempt): Performance: Reduced lock contention in BsonSerializer.LookupActualType #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 66 additions & 53 deletions src/MongoDB.Bson/Serialization/BsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/

using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -41,7 +42,7 @@ public static class BsonSerializer
private static HashSet<Type> __discriminatedTypes = new HashSet<Type>();
private static BsonSerializerRegistry __serializerRegistry;
private static TypeMappingSerializationProvider __typeMappingSerializationProvider;
private static HashSet<Type> __typesWithRegisteredKnownTypes = new HashSet<Type>();
private static Hashtable __typesWithRegisteredKnownTypes = new Hashtable(EqualityComparer<Type>.Default);

private static bool __useNullIdChecker = false;
private static bool __useZeroIdChecker = false;
Expand Down Expand Up @@ -275,7 +276,20 @@ public static object Deserialize(TextReader textReader, Type nominalType, Action
public static bool IsTypeDiscriminated(Type type)
{
var typeInfo = type.GetTypeInfo();
return typeInfo.IsInterface || __discriminatedTypes.Contains(type);
if (typeInfo.IsInterface)
{
return true;
}

__configLock.EnterReadLock();
try
{
return __discriminatedTypes.Contains(type);
}
finally
{
__configLock.ExitReadLock();
}
}

/// <summary>
Expand All @@ -294,63 +308,67 @@ public static Type LookupActualType(Type nominalType, BsonValue discriminator)
// note: EnsureKnownTypesAreRegistered handles its own locking so call from outside any lock
EnsureKnownTypesAreRegistered(nominalType);

HashSet<Type> hashSet;

__configLock.EnterReadLock();
try
{
Type actualType = null;
__discriminators.TryGetValue(discriminator, out hashSet);
}
finally
{
__configLock.ExitReadLock();
}

HashSet<Type> hashSet;
var nominalTypeInfo = nominalType.GetTypeInfo();
if (__discriminators.TryGetValue(discriminator, out hashSet))
Type actualType = null;
var nominalTypeInfo = nominalType.GetTypeInfo();

if (hashSet != null)
{
// The values inside __discriminator are expected to be immutable so it is safe to enumerate the contained HashSet<Type> outside the lock
foreach (var type in hashSet)
{
foreach (var type in hashSet)
if (nominalTypeInfo.IsAssignableFrom(type))
{
if (nominalTypeInfo.IsAssignableFrom(type))
if (actualType == null)
{
if (actualType == null)
{
actualType = type;
}
else
{
string message = string.Format("Ambiguous discriminator '{0}'.", discriminator);
throw new BsonSerializationException(message);
}
actualType = type;
}
else
{
string message = string.Format("Ambiguous discriminator '{0}'.", discriminator);
throw new BsonSerializationException(message);
}
}

// no need for additional checks, we found the right type
if (actualType != null)
{
return actualType;
}
}

if (discriminator.IsString)
// no need for additional checks, we found the right type
if (actualType != null)
{
actualType = TypeNameDiscriminator.GetActualType(discriminator.AsString); // see if it's a Type name
}

if (actualType == null)
{
string message = string.Format("Unknown discriminator value '{0}'.", discriminator);
throw new BsonSerializationException(message);
return actualType;
}
}

if (!nominalTypeInfo.IsAssignableFrom(actualType))
{
string message = string.Format(
"Actual type {0} is not assignable to expected type {1}.",
actualType.FullName, nominalType.FullName);
throw new BsonSerializationException(message);
}
if (discriminator.IsString)
{
actualType = TypeNameDiscriminator.GetActualType(discriminator.AsString); // see if it's a Type name
}

return actualType;
if (actualType == null)
{
string message = string.Format("Unknown discriminator value '{0}'.", discriminator);
throw new BsonSerializationException(message);
}
finally

if (!nominalTypeInfo.IsAssignableFrom(actualType))
{
__configLock.ExitReadLock();
string message = string.Format(
"Actual type {0} is not assignable to expected type {1}.",
actualType.FullName, nominalType.FullName);
throw new BsonSerializationException(message);
}

return actualType;
}

/// <summary>
Expand Down Expand Up @@ -535,7 +553,10 @@ public static void RegisterDiscriminator(Type type, BsonValue discriminator)

if (!hashSet.Contains(type))
{
// clone existing HashSet to not interfere with not synchronized parallel reads to the old instance
hashSet = new HashSet<Type>(hashSet);
hashSet.Add(type);
__discriminators[discriminator] = hashSet;

// mark all base types as discriminated (so we know that it's worth reading a discriminator)
for (var baseType = typeInfo.BaseType; baseType != null; baseType = baseType.GetTypeInfo().BaseType)
Expand Down Expand Up @@ -679,23 +700,15 @@ public static void Serialize(
// internal static methods
internal static void EnsureKnownTypesAreRegistered(Type nominalType)
{
__configLock.EnterReadLock();
try
if (__typesWithRegisteredKnownTypes.ContainsKey(nominalType))
{
if (__typesWithRegisteredKnownTypes.Contains(nominalType))
{
return;
}
}
finally
{
__configLock.ExitReadLock();
return;
}

__configLock.EnterWriteLock();
try
{
if (!__typesWithRegisteredKnownTypes.Contains(nominalType))
if (!__typesWithRegisteredKnownTypes.ContainsKey(nominalType))
{
// only call LookupClassMap for classes with a BsonKnownTypesAttribute
#if NET452
Expand All @@ -709,7 +722,7 @@ internal static void EnsureKnownTypesAreRegistered(Type nominalType)
LookupSerializer(nominalType);
}

__typesWithRegisteredKnownTypes.Add(nominalType);
__typesWithRegisteredKnownTypes[nominalType] = null /* we don't care about the value */;
}
}
finally
Expand Down