Skip to content

Commit

Permalink
Remove legacy light-up code from System.Reflection.Metadata et.al. (d…
Browse files Browse the repository at this point in the history
…otnet#56587)

* Remove legacy light-up code for the pointer-based Encoding.GetString.

* Remove legacy light-up code for the memory-mapped file block code.

* Remove FileStreamLightUp.IsFileStream and the need for it.

* Address PR feedback.

* Use hardware-accelerated population count where available.

* Remove a try/finally with an empty try where CERs are not supported.

* Use named arguments when constructing a memory-mapped file.
  • Loading branch information
teo-tsirpanis authored Sep 16, 2021
1 parent 4bf4300 commit db9c3f1
Show file tree
Hide file tree
Showing 17 changed files with 63 additions and 402 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<DefaultLanguage>en-US</DefaultLanguage>
Expand Down Expand Up @@ -110,14 +110,12 @@ System.Reflection.PortableExecutable.ManagedPEBuilder</PackageDescription>
<Compile Include="System\Reflection\Internal\Utilities\EmptyArray.cs" />
<Compile Include="System\Reflection\Internal\Utilities\EncodingHelper.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\EncodingHelper.netcoreapp.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\FileStreamReadLightUp.netcoreapp.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\FileStreamReadLightUp.netstandard2.0.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\StreamExtensions.netcoreapp.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\StreamExtensions.netstandard2.0.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\Hash.cs" />
<Compile Include="System\Reflection\Internal\Utilities\ImmutableByteArrayInterop.cs" />
<Compile Include="System\Reflection\Internal\Utilities\ImmutableMemoryStream.cs" />
<Compile Include="System\Reflection\Internal\Utilities\LightUpHelper.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
<Compile Include="System\Reflection\Internal\Utilities\MemoryBlock.cs" />
<Compile Include="System\Reflection\Internal\Utilities\MemoryMapLightUp.cs" />
<Compile Include="System\Reflection\Internal\Utilities\PooledStringBuilder.cs" />
<Compile Include="System\Reflection\Internal\Utilities\ObjectPool`1.cs" />
<Compile Include="System\Reflection\Internal\Utilities\ReadOnlyUnmanagedMemoryStream.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ internal unsafe sealed class MemoryMappedFileBlock : AbstractMemoryBlock
{
private sealed class DisposableData : CriticalDisposableObject
{
private IDisposable? _accessor; // MemoryMappedViewAccessor
// Usually a MemoryMappedViewAccessor, but kept
// as an IDisposable for better testability.
private IDisposable? _accessor;
private SafeBuffer? _safeBuffer;
private byte* _pointer;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.IO.MemoryMappedFiles;
using System.Threading;

namespace System.Reflection.Internal
Expand All @@ -28,24 +29,21 @@ internal sealed class StreamMemoryBlockProvider : MemoryBlockProvider

private readonly bool _leaveOpen;
private bool _useMemoryMap;
private readonly bool _isFileStream;

private readonly long _imageStart;
private readonly int _imageSize;

// MemoryMappedFile
private IDisposable? _lazyMemoryMap;
private MemoryMappedFile? _lazyMemoryMap;

public StreamMemoryBlockProvider(Stream stream, long imageStart, int imageSize, bool isFileStream, bool leaveOpen)
public StreamMemoryBlockProvider(Stream stream, long imageStart, int imageSize, bool leaveOpen)
{
Debug.Assert(stream.CanSeek && stream.CanRead);
_stream = stream;
_streamGuard = new object();
_imageStart = imageStart;
_imageSize = imageSize;
_leaveOpen = leaveOpen;
_isFileStream = isFileStream;
_useMemoryMap = isFileStream && MemoryMapLightUp.IsAvailable;
_useMemoryMap = stream is FileStream;
}

protected override void Dispose(bool disposing)
Expand All @@ -68,7 +66,7 @@ public override int Size
}

/// <exception cref="IOException">Error reading from the stream.</exception>
internal static unsafe NativeHeapMemoryBlock ReadMemoryBlockNoLock(Stream stream, bool isFileStream, long start, int size)
internal static unsafe NativeHeapMemoryBlock ReadMemoryBlockNoLock(Stream stream, long start, int size)
{
var block = new NativeHeapMemoryBlock(size);
bool fault = true;
Expand All @@ -78,7 +76,7 @@ internal static unsafe NativeHeapMemoryBlock ReadMemoryBlockNoLock(Stream stream

int bytesRead = 0;

if (!isFileStream || (bytesRead = FileStreamReadLightUp.ReadFile(stream, block.Pointer, size)) != size)
if ((bytesRead = stream.Read(block.Pointer, size)) != size)
{
stream.CopyTo(block.Pointer + bytesRead, size - bytesRead);
}
Expand Down Expand Up @@ -113,7 +111,7 @@ protected override AbstractMemoryBlock GetMemoryBlockImpl(int start, int size)

lock (_streamGuard)
{
return ReadMemoryBlockNoLock(_stream!, _isFileStream, absoluteStart, size);
return ReadMemoryBlockNoLock(_stream!, absoluteStart, size);
}
}

Expand All @@ -129,12 +127,25 @@ private unsafe bool TryCreateMemoryMappedFileBlock(long start, int size, [NotNul
if (_lazyMemoryMap == null)
{
// leave the underlying stream open. It will be closed by the Dispose method.
IDisposable newMemoryMap;
MemoryMappedFile newMemoryMap;

// CreateMemoryMap might modify the stream (calls FileStream.Flush)
lock (_streamGuard)
{
newMemoryMap = MemoryMapLightUp.CreateMemoryMap(_stream);
try
{
newMemoryMap =
MemoryMappedFile.CreateFromFile(
fileStream: (FileStream)_stream,
mapName: null,
capacity: 0,
access: MemoryMappedFileAccess.Read,
inheritability: HandleInheritability.None,
leaveOpen: true);
} catch (UnauthorizedAccessException e)
{
throw new IOException(e.Message, e);
}
}

if (newMemoryMap == null)
Expand All @@ -149,20 +160,14 @@ private unsafe bool TryCreateMemoryMappedFileBlock(long start, int size, [NotNul
}
}

IDisposable accessor = MemoryMapLightUp.CreateViewAccessor(_lazyMemoryMap, start, size);
MemoryMappedViewAccessor accessor = _lazyMemoryMap.CreateViewAccessor(start, size, MemoryMappedFileAccess.Read);
if (accessor == null)
{
block = null;
return false;
}

if (!MemoryMapLightUp.TryGetSafeBufferAndPointerOffset(accessor, out var safeBuffer, out long offset))
{
block = null;
return false;
}

block = new MemoryMappedFileBlock(accessor, safeBuffer, offset, size);
block = new MemoryMappedFileBlock(accessor, accessor.SafeMemoryMappedViewHandle, accessor.PointerOffset, size);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
#if NETCOREAPP
using System.Numerics;
#endif

namespace System.Reflection.Internal
{
Expand All @@ -14,23 +17,31 @@ internal static int CountBits(int v)

internal static int CountBits(uint v)
{
#if NETCOREAPP
return BitOperations.PopCount(v);
#else
unchecked
{
v = v - ((v >> 1) & 0x55555555u);
v = (v & 0x33333333u) + ((v >> 2) & 0x33333333u);
return (int)((v + (v >> 4) & 0xF0F0F0Fu) * 0x1010101u) >> 24;
}
#endif
}

internal static int CountBits(ulong v)
{
#if NETCOREAPP
return BitOperations.PopCount(v);
#else
const ulong Mask01010101 = 0x5555555555555555UL;
const ulong Mask00110011 = 0x3333333333333333UL;
const ulong Mask00001111 = 0x0F0F0F0F0F0F0F0FUL;
const ulong Mask00000001 = 0x0101010101010101UL;
v = v - ((v >> 1) & Mask01010101);
v = (v & Mask00110011) + ((v >> 2) & Mask00110011);
return (int)(unchecked(((v + (v >> 4)) & Mask00001111) * Mask00000001) >> 56);
#endif
}

internal static uint Align(uint position, uint alignment)
Expand Down
Loading

0 comments on commit db9c3f1

Please sign in to comment.