From 39a2911db4f33186991791e3885e81ab20979605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 2 Oct 2015 12:41:12 -0700 Subject: [PATCH 1/2] Move thread safe flag manipulation into a helper --- src/ILToNative/src/ILToNative.csproj | 3 ++ src/TypeSystem/src/Common/FieldLayout.cs | 56 +++++++++----------- src/TypeSystem/src/Common/ThreadSafeFlags.cs | 38 +++++++++++++ src/TypeSystem/src/Ecma/EcmaField.cs | 15 ++---- src/TypeSystem/src/TypeSystem.csproj | 1 + 5 files changed, 70 insertions(+), 43 deletions(-) create mode 100644 src/TypeSystem/src/Common/ThreadSafeFlags.cs diff --git a/src/ILToNative/src/ILToNative.csproj b/src/ILToNative/src/ILToNative.csproj index 4e2eef56c41..9a64e4a2aa1 100644 --- a/src/ILToNative/src/ILToNative.csproj +++ b/src/ILToNative/src/ILToNative.csproj @@ -34,6 +34,9 @@ + + TypeSystem\ThreadSafeFlags.cs + diff --git a/src/TypeSystem/src/Common/FieldLayout.cs b/src/TypeSystem/src/Common/FieldLayout.cs index 93da4f4c387..a2653d2e0c6 100644 --- a/src/TypeSystem/src/Common/FieldLayout.cs +++ b/src/TypeSystem/src/Common/FieldLayout.cs @@ -4,7 +4,6 @@ using System; using System.Linq; using Debug = System.Diagnostics.Debug; -using Interlocked = System.Threading.Interlocked; namespace Internal.TypeSystem { @@ -458,7 +457,7 @@ private class StaticBlockInfo public StaticsBlock ThreadStatics; } - volatile int _fieldLayoutFlags; + ThreadSafeFlags _fieldLayoutFlags; int _instanceFieldSize; int _instanceByteCount; @@ -471,16 +470,11 @@ public bool ContainsPointers { get { - if ((_fieldLayoutFlags & FieldLayoutFlags.HasContainsPointers) == 0) + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.HasContainsPointers)) { - var flagsToAdd = FieldLayoutFlags.HasContainsPointers; - - if (ComputeTypeContainsPointers()) - flagsToAdd |= FieldLayoutFlags.ContainsPointers; - - EnableFieldLayoutFlags(flagsToAdd); + ComputeTypeContainsPointers(); } - return (_fieldLayoutFlags & FieldLayoutFlags.ContainsPointers) != 0; + return _fieldLayoutFlags.HasFlags(FieldLayoutFlags.ContainsPointers); } } @@ -488,7 +482,7 @@ public int InstanceFieldSize { get { - if ((_fieldLayoutFlags & FieldLayoutFlags.HasInstanceFieldLayout) == 0) + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.HasInstanceFieldLayout)) { ComputeInstanceFieldLayout(); } @@ -500,7 +494,7 @@ public int InstanceFieldAlignment { get { - if ((_fieldLayoutFlags & FieldLayoutFlags.HasInstanceFieldLayout) == 0) + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.HasInstanceFieldLayout)) { ComputeInstanceFieldLayout(); } @@ -512,7 +506,7 @@ public int InstanceByteCount { get { - if ((_fieldLayoutFlags & FieldLayoutFlags.HasInstanceFieldLayout) == 0) + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.HasInstanceFieldLayout)) { ComputeInstanceFieldLayout(); } @@ -524,7 +518,7 @@ public int NonGCStaticFieldSize { get { - if ((_fieldLayoutFlags & FieldLayoutFlags.HasStaticFieldLayout) == 0) + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.HasStaticFieldLayout)) { ComputeStaticFieldLayout(); } @@ -536,7 +530,7 @@ public int NonGCStaticFieldAlignment { get { - if ((_fieldLayoutFlags & FieldLayoutFlags.HasStaticFieldLayout) == 0) + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.HasStaticFieldLayout)) { ComputeStaticFieldLayout(); } @@ -558,7 +552,7 @@ internal void ComputeInstanceFieldLayout() fieldAndOffset.Field.InitializeOffset(fieldAndOffset.Offset); } - EnableFieldLayoutFlags(FieldLayoutFlags.HasInstanceFieldLayout); + _fieldLayoutFlags.AddFlags(FieldLayoutFlags.HasInstanceFieldLayout); } internal void ComputeStaticFieldLayout() @@ -584,13 +578,18 @@ internal void ComputeStaticFieldLayout() } } - EnableFieldLayoutFlags(FieldLayoutFlags.HasStaticFieldLayout); + _fieldLayoutFlags.AddFlags(FieldLayoutFlags.HasStaticFieldLayout); } - private bool ComputeTypeContainsPointers() + private void ComputeTypeContainsPointers() { + int flagsToAdd = FieldLayoutFlags.HasContainsPointers; + if (!IsValueType && HasBaseType && BaseType.ContainsPointers) - return true; + { + _fieldLayoutFlags.AddFlags(flagsToAdd | FieldLayoutFlags.ContainsPointers); + return; + } foreach (var field in GetFields()) { @@ -606,24 +605,19 @@ private bool ComputeTypeContainsPointers() // case TypeFlags.MethodGenericParameter? case TypeFlags.GenericParameter: case TypeFlags.ByRef: - return true; + _fieldLayoutFlags.AddFlags(flagsToAdd | FieldLayoutFlags.ContainsPointers); + return; case TypeFlags.ValueType: if (((MetadataType)fieldType).ContainsPointers) - return true; + { + _fieldLayoutFlags.AddFlags(flagsToAdd | FieldLayoutFlags.ContainsPointers); + return; + } break; } } - return false; - } - - private void EnableFieldLayoutFlags(int flagsToAdd) - { - var originalFlags = _fieldLayoutFlags; - while (Interlocked.CompareExchange(ref _fieldLayoutFlags, originalFlags | flagsToAdd, originalFlags) != originalFlags) - { - originalFlags = _fieldLayoutFlags; - } + _fieldLayoutFlags.AddFlags(flagsToAdd); } } diff --git a/src/TypeSystem/src/Common/ThreadSafeFlags.cs b/src/TypeSystem/src/Common/ThreadSafeFlags.cs new file mode 100644 index 00000000000..8a022987a63 --- /dev/null +++ b/src/TypeSystem/src/Common/ThreadSafeFlags.cs @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Runtime.CompilerServices; +using Interlocked = System.Threading.Interlocked; + +namespace Internal.TypeSystem +{ + struct ThreadSafeFlags + { + private volatile int _value; + + public int Value + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get + { + return _value; + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool HasFlags(int value) + { + return (_value & value) == value; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void AddFlags(int flagsToAdd) + { + var originalFlags = _value; + while (Interlocked.CompareExchange(ref _value, originalFlags | flagsToAdd, originalFlags) != originalFlags) + { + originalFlags = _value; + } + } + } +} diff --git a/src/TypeSystem/src/Ecma/EcmaField.cs b/src/TypeSystem/src/Ecma/EcmaField.cs index 41a3dbc5879..c8cbfb6b0ba 100644 --- a/src/TypeSystem/src/Ecma/EcmaField.cs +++ b/src/TypeSystem/src/Ecma/EcmaField.cs @@ -9,8 +9,6 @@ using Internal.TypeSystem; -using Interlocked = System.Threading.Interlocked; - namespace Internal.TypeSystem.Ecma { public sealed class EcmaField : FieldDesc @@ -31,7 +29,7 @@ static class FieldFlags FieldDefinitionHandle _handle; // Cached values - volatile int _fieldFlags; + ThreadSafeFlags _fieldFlags; TypeDesc _fieldType; string _name; @@ -152,14 +150,7 @@ private int InitializeFieldFlags(int mask) Debug.Assert((flags & mask) != 0); - // Atomically update flags - var originalFlags = _fieldFlags; - while (Interlocked.CompareExchange(ref _fieldFlags, (int)(originalFlags | flags), originalFlags) != originalFlags) - { - originalFlags = _fieldFlags; - } - - _fieldFlags |= flags; + _fieldFlags.AddFlags(flags); return flags & mask; } @@ -167,7 +158,7 @@ private int InitializeFieldFlags(int mask) [MethodImpl(MethodImplOptions.AggressiveInlining)] private int GetFieldFlags(int mask) { - int flags = _fieldFlags & mask; + int flags = _fieldFlags.Value & mask; if (flags != 0) return flags; return InitializeFieldFlags(mask); diff --git a/src/TypeSystem/src/TypeSystem.csproj b/src/TypeSystem/src/TypeSystem.csproj index 90106352abf..55de7def758 100644 --- a/src/TypeSystem/src/TypeSystem.csproj +++ b/src/TypeSystem/src/TypeSystem.csproj @@ -44,6 +44,7 @@ + From 6745182dc84e87d1008becfedbec14e79341d1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 2 Oct 2015 12:52:21 -0700 Subject: [PATCH 2/2] Move things around --- src/ILToNative/src/ILToNative.csproj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ILToNative/src/ILToNative.csproj b/src/ILToNative/src/ILToNative.csproj index 9a64e4a2aa1..387279863e8 100644 --- a/src/ILToNative/src/ILToNative.csproj +++ b/src/ILToNative/src/ILToNative.csproj @@ -34,9 +34,6 @@ - - TypeSystem\ThreadSafeFlags.cs - @@ -96,6 +93,9 @@ TypeSystem\TargetDetails.cs + + TypeSystem\ThreadSafeFlags.cs + TypeSystem\TypeCast.cs