Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Improve Dictionary TryGetValue size/perfomance #27195

Merged
merged 8 commits into from
Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Add NullRef methods to Unsafe
  • Loading branch information
benaadams committed Oct 15, 2019
commit cb70550940c7fdfdbe11f711db2fee821dfcd6df
Original file line number Diff line number Diff line change
Expand Up @@ -387,5 +387,65 @@ public static IntPtr ByteOffset<T>(ref T origin, ref T target)
{
throw new PlatformNotSupportedException();
}

/// <summary>
/// Returns a by-ref to a null reference of type <typeparamref name="T"/>.
/// Note this is the address of the by-ref not what the ref points to.
benaadams marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
[Intrinsic]
[NonVersionable]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static ref T NullRef<T>()
benaadams marked this conversation as resolved.
Show resolved Hide resolved
{
#if CORECLR
benaadams marked this conversation as resolved.
Show resolved Hide resolved
throw new PlatformNotSupportedException();
// ldnull
// ret
#else
return ref Unsafe.AsRef<T>(null);
#endif
}

/// <summary>
/// Returns if a given by-ref of type <typeparamref name="T"/> is a null reference.
/// Note this is the address of the by-ref not what the ref points to.
/// </summary>
[Intrinsic]
[NonVersionable]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool IsNullRef<T>(ref T source)
{
#if CORECLR
throw new PlatformNotSupportedException();
// ldarg.0
// ldc.i4.0
// conv.u
// ceq
// ret
#else
return Unsafe.AsPointer(ref source) == null;
#endif
}

/// <summary>
/// Returns if a given by-ref of type <typeparamref name="T"/> is not a null reference.
/// Note this is the address of the by-ref not what the ref points to.
/// </summary>
[Intrinsic]
[NonVersionable]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool IsNotNullRef<T>(ref T source)
benaadams marked this conversation as resolved.
Show resolved Hide resolved
{
#if CORECLR
throw new PlatformNotSupportedException();
// ldarg.0
// ldc.i4.0
// conv.u
// cgt.un
// ret
#else
return Unsafe.AsPointer(ref source) != null;
#endif
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Runtime.Serialization;
using System.Runtime.InteropServices;

using Internal.Runtime.CompilerServices;

Expand Down Expand Up @@ -39,15 +38,14 @@ internal enum InsertionBehavior : byte
[TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
public class Dictionary<TKey, TValue> : IDictionary<TKey, TValue>, IDictionary, IReadOnlyDictionary<TKey, TValue>, ISerializable, IDeserializationCallback where TKey : notnull
{
[StructLayout(LayoutKind.Auto)]
private struct Entry
{
public uint hashCode;
public TKey key; // Key of entry
// 0-based index of next entry in chain: -1 means end of chain
// also encodes whether this entry _itself_ is part of the free list by changing sign and subtracting 3,
// so -2 means end of free list, -3 means index 0 but on free list, -4 means index 1 but on free list, etc.
public int next;
public TKey key; // Key of entry
public TValue value; // Value of entry
}

Expand Down Expand Up @@ -168,12 +166,12 @@ protected Dictionary(SerializationInfo info, StreamingContext context)

IEnumerable<TValue> IReadOnlyDictionary<TKey, TValue>.Values => _values ??= new ValueCollection(this);

public unsafe TValue this[TKey key]
public TValue this[TKey key]
{
get
{
ref TValue value = ref FindValue(key);
if (Unsafe.AsPointer(ref value) != null)
if (Unsafe.IsNotNullRef(ref value))
{
return value;
}
Expand All @@ -196,20 +194,20 @@ public void Add(TKey key, TValue value)
void ICollection<KeyValuePair<TKey, TValue>>.Add(KeyValuePair<TKey, TValue> keyValuePair)
=> Add(keyValuePair.Key, keyValuePair.Value);

unsafe bool ICollection<KeyValuePair<TKey, TValue>>.Contains(KeyValuePair<TKey, TValue> keyValuePair)
bool ICollection<KeyValuePair<TKey, TValue>>.Contains(KeyValuePair<TKey, TValue> keyValuePair)
{
ref TValue value = ref FindValue(keyValuePair.Key);
if (Unsafe.AsPointer(ref value) != null && EqualityComparer<TValue>.Default.Equals(value, keyValuePair.Value))
if (Unsafe.IsNotNullRef(ref value) && EqualityComparer<TValue>.Default.Equals(value, keyValuePair.Value))
{
return true;
}
return false;
}

unsafe bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> keyValuePair)
bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> keyValuePair)
{
ref TValue value = ref FindValue(keyValuePair.Key);
if (Unsafe.AsPointer(ref value) != null && EqualityComparer<TValue>.Default.Equals(value, keyValuePair.Value))
if (Unsafe.IsNotNullRef(ref value) && EqualityComparer<TValue>.Default.Equals(value, keyValuePair.Value))
{
Remove(keyValuePair.Key);
return true;
Expand All @@ -236,7 +234,7 @@ public void Clear()

[MethodImpl(MethodImplOptions.AggressiveInlining)]
benaadams marked this conversation as resolved.
Show resolved Hide resolved
public unsafe bool ContainsKey(TKey key)
benaadams marked this conversation as resolved.
Show resolved Hide resolved
=> Unsafe.AsPointer(ref FindValue(key)) != null;
=> Unsafe.IsNotNullRef(ref FindValue(key));

public bool ContainsValue(TValue value)
{
Expand Down Expand Up @@ -334,7 +332,7 @@ private unsafe ref TValue FindValue(TKey key)
}

int[]? buckets = _buckets;
ref Entry entry = ref Unsafe.AsRef<Entry>(null);
ref Entry entry = ref Unsafe.NullRef<Entry>();
if (buckets != null)
{
Debug.Assert(_entries != null, "expected entries to be != null");
Expand Down Expand Up @@ -450,7 +448,7 @@ private unsafe ref TValue FindValue(TKey key)
Return:
return ref value;
ReturnNotFound:
value = ref Unsafe.AsRef<TValue>(null);
value = ref Unsafe.NullRef<TValue>();
goto Return;
}

Expand Down Expand Up @@ -898,7 +896,7 @@ public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value)
public unsafe bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value)
benaadams marked this conversation as resolved.
Show resolved Hide resolved
{
ref TValue valRef = ref FindValue(key);
if (Unsafe.AsPointer(ref valRef) != null)
if (Unsafe.IsNotNullRef(ref valRef))
{
value = valRef;
return true;
Expand Down Expand Up @@ -1059,14 +1057,14 @@ public void TrimExcess(int capacity)

ICollection IDictionary.Values => (ICollection)Values;

unsafe object? IDictionary.this[object key]
object? IDictionary.this[object key]
{
get
{
if (IsCompatibleKey(key))
{
ref TValue value = ref FindValue((TKey)key);
if (Unsafe.AsPointer(ref value) != null)
if (Unsafe.IsNotNullRef(ref value))
{
return value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/System.Private.CoreLib/shared/System/Memory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ public unsafe Span<T> Span
// in which case that's the dangerous operation performed by the dev, and we're just following
// suit here to make it work as best as possible.

ref T refToReturn = ref Unsafe.AsRef<T>(null);
ref T refToReturn = ref Unsafe.NullRef<T>();
int lengthOfUnderlyingSpan = 0;

// Copy this field into a local so that it can't change out from under us mid-operation.
Expand Down
2 changes: 1 addition & 1 deletion src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public unsafe ReadOnlySpan<T> Span
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get
{
ref T refToReturn = ref Unsafe.AsRef<T>(null);
ref T refToReturn = ref Unsafe.NullRef<T>();
int lengthOfUnderlyingSpan = 0;

// Copy this field into a local so that it can't change out from under us mid-operation.
Expand Down
4 changes: 2 additions & 2 deletions src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ public ref readonly T this[int index]
/// It can be used for pinning and is required to support the use of span within a fixed statement.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public unsafe ref readonly T GetPinnableReference()
public ref readonly T GetPinnableReference()
{
// Ensure that the native code has just one forward branch that is predicted-not-taken.
ref T ret = ref Unsafe.AsRef<T>(null);
ref T ret = ref Unsafe.NullRef<T>();
if (_length != 0) ret = ref _pointer.Value;
return ref ret;
}
Expand Down
4 changes: 2 additions & 2 deletions src/System.Private.CoreLib/shared/System/Span.Fast.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ public ref T this[int index]
/// It can be used for pinning and is required to support the use of span within a fixed statement.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public unsafe ref T GetPinnableReference()
public ref T GetPinnableReference()
{
// Ensure that the native code has just one forward branch that is predicted-not-taken.
ref T ret = ref Unsafe.AsRef<T>(null);
ref T ret = ref Unsafe.NullRef<T>();
if (_length != 0) ret = ref _pointer.Value;
return ref ret;
}
Expand Down
36 changes: 36 additions & 0 deletions src/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7251,6 +7251,42 @@ bool getILIntrinsicImplementationForUnsafe(MethodDesc * ftn,
methInfo->options = (CorInfoOptions)0;
return true;
}
else if (tk == MscorlibBinder::GetMethod(METHOD__UNSAFE__BYREF_NULLREF)->GetMemberDef())
{
static const BYTE ilcode[] = { CEE_LDNULL, CEE_RET };
benaadams marked this conversation as resolved.
Show resolved Hide resolved
methInfo->ILCode = const_cast<BYTE*>(ilcode);
methInfo->ILCodeSize = sizeof(ilcode);
methInfo->maxStack = 1;
methInfo->EHcount = 0;
methInfo->options = (CorInfoOptions)0;
return true;
}
else if (tk == MscorlibBinder::GetMethod(METHOD__UNSAFE__BYREF_IS_NOTNULL)->GetMemberDef())
{
// 'ldnull' opcode would produce type o, and we can't compare & against o (ECMA-335, Table III.4).
// However, we can compare & against native int, so we'll use that instead.

static const BYTE ilcode[] = { CEE_LDARG_0, CEE_LDC_I4_0, CEE_CONV_U, CEE_PREFIX1, (CEE_CGT_UN & 0xFF), CEE_RET };
methInfo->ILCode = const_cast<BYTE*>(ilcode);
methInfo->ILCodeSize = sizeof(ilcode);
methInfo->maxStack = 2;
methInfo->EHcount = 0;
methInfo->options = (CorInfoOptions)0;
return true;
}
else if (tk == MscorlibBinder::GetMethod(METHOD__UNSAFE__BYREF_IS_NULL)->GetMemberDef())
{
// 'ldnull' opcode would produce type o, and we can't compare & against o (ECMA-335, Table III.4).
// However, we can compare & against native int, so we'll use that instead.

static const BYTE ilcode[] = { CEE_LDARG_0, CEE_LDC_I4_0, CEE_CONV_U, CEE_PREFIX1, (CEE_CEQ & 0xFF), CEE_RET };
methInfo->ILCode = const_cast<BYTE*>(ilcode);
methInfo->ILCodeSize = sizeof(ilcode);
methInfo->maxStack = 2;
methInfo->EHcount = 0;
methInfo->options = (CorInfoOptions)0;
return true;
}
else if (tk == MscorlibBinder::GetMethod(METHOD__UNSAFE__BYREF_INIT_BLOCK_UNALIGNED)->GetMemberDef())
{
static const BYTE ilcode[] = { CEE_LDARG_0, CEE_LDARG_1, CEE_LDARG_2, CEE_PREFIX1, (CEE_UNALIGNED & 0xFF), 0x01, CEE_PREFIX1, (CEE_INITBLK & 0xFF), CEE_RET };
Expand Down
3 changes: 3 additions & 0 deletions src/vm/mscorlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,9 @@ DEFINE_METHOD(JIT_HELPERS, ENUM_COMPARE_TO, EnumCompareTo, NoSig

DEFINE_CLASS(UNSAFE, InternalCompilerServices, Unsafe)
DEFINE_METHOD(UNSAFE, AS_POINTER, AsPointer, NoSig)
DEFINE_METHOD(UNSAFE, BYREF_IS_NULL, IsNullRef, NoSig)
DEFINE_METHOD(UNSAFE, BYREF_IS_NOTNULL, IsNotNullRef, NoSig)
DEFINE_METHOD(UNSAFE, BYREF_NULLREF, NullRef, NoSig)
DEFINE_METHOD(UNSAFE, AS_REF_IN, AsRef, GM_RefT_RetRefT)
DEFINE_METHOD(UNSAFE, AS_REF_POINTER, AsRef, GM_VoidPtr_RetRefT)
DEFINE_METHOD(UNSAFE, SIZEOF, SizeOf, NoSig)
Expand Down