[API Proposal]: Embrace spans in System.Reflection.Metadata.

[teo-tsirpanis]

Background and motivation

System.Reflection.Metadata is a pretty perormance-oriented library but its API lacks methods that accept spans or facilitate zero-copy memory access for some scenarios. I propose additional span overloads for methods that work with memory buffers. Some of these APIs can now be implemented efficiently thanks to the work started in #81059.

API Proposal

namespace System.Reflection.Metadata;

public class BlobContentId
{
    public BlobContentId(ReadOnlySpan<byte> id);
    public static BlobContentId FromHash(ReadOnlySpan<byte> hashCode);
}

public struct BlobReader
{
    public ReadOnlySpan<byte> RemainingSpan { get; }
    public ReadOnlySpan<byte> Span { get; }
}

public struct BlobWriter
{
    public void WriteBytes(ReadOnlySpan<byte> buffer);
}

public struct MetadataReader
{
    public BlobReader GetBlobReader(UserStringHandle handle);
    // Taken from #103169 and expanded
    public bool TryGetBlob(BlobHandle handle, Span<byte> span, out int length);
    public bool TryGetString(StringHandle handle, Span<char> span, out int length);
    public bool TryGetString(DocumentNameBlobHandle handle, Span<char> span, out int length);
    public bool TryGetString(NamespaceDefinitionHandle handle, Span<char> span, out int length);
    public bool TryGetString(UserStringHandle handle, Span<char> span, out int length);
}

namespace System.Reflection.Metadata.Ecma335;

public struct ArrayShapeEncoder
{
    public void Shape(int rank, ReadOnlySpan<int> sizes, ReadOnlySpan<int> lowerBounds) { }
}

public sealed class MetadataBuilder
{
    // These APIs will not allocate if the blob/string already exists.
    public BlobHandle GetOrAddBlob(ReadOnlySpan<byte> value);
    public BlobHandle GetOrAddBlobUTF8(ReadOnlySpan<char> value, bool allowUnpairedSurrogates = true);
    public BlobHandle GetOrAddBlobUTF16(ReadOnlySpan<char> value);
    public BlobHandle GetOrAddDocumentName(ReadOnlySpan<char> value);
    public StringHandle GetOrAddString(ReadOnlySpan<char> value);
    public UserStringHandle GetOrAddUserString(ReadOnlySpan<char> value);
}

public readonly partial struct PermissionSetEncoder
{
    public PermissionSetEncoder AddPermission(string typeName, ReadOnlySpan<byte> encodedArguments);
}

namespace System.Reflection.PortableExecutable;

public sealed partial class DebugDirectoryBuilder
{
    public void AddEntry(DebugDirectoryEntryType type, uint version, uint stamp, ReadOnlySpan<byte> data);
    // Existing API, add allows ref struct to TData on supported frameworks
    public void AddEntry<TData>(DebugDirectoryEntryType type, uint version, uint stamp, TData data, System.Action<BlobBuilder, TData> dataSerializer)
#if NET9_0_OR_HIGHER
        where TData : allows ref struct
#endif
    ;
    public void AddPdbChecksumEntry(string algorithmName, ReadOnlySpan<byte> checksum);
}

API Usage

The proposed APIs correspond to existing ones that work with strings and (immutable) byte arrays. Their usage will be similar.

Alternative Designs

• The APIs in ArrayShapeEncoder and PermissionSetEncoder are not strictly necessary (especially the latter) and could be omitted.
• The TryGet*** APIs are not necessary either; a user can call GetBlobReader and have zero-copy access to the raw bytes.
  • However, when called on virtual blob and string handles (used for WinRT projections), GetBlobReader returns a buffer pointing to newly allocated and pinned memory, which is not the best for performance, but there is room for improvement.
  • Namespace definition and document name blob handles don't have a raw byte representation, so the TryGet*** APIs will be needed to avoid allocations.
• A previous iteration of this proposal suggested some additional APIs for BlobReader to read byte buffers and strings into spans, but I removed them because this is something users can already do by getting the reader's underlying pointer or span.

Risks

APIs to get a span from a BlobReader might be considered unsafe because BlobReader wraps unmanaged memory and the span will not ensure the memory is kept alive. But this is a problem with SRM in general.

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

System.Reflection.Metadata is a pretty perormance-oriented library but its API lacks methods that accept spans. I propose additional span overloads for methods that work with memory buffers. Some of these APIs can now be implemented efficiently thanks to the work started in #81059.

API Proposal

namespace System.Reflection.Metadata;

public class BlobBuilder
{
    public void WriteBytes(ReadOnlySpan<byte> buffer);
}

public class BlobContentId
{
    public BlobContentId(ReadOnlySpan<byte> id);
    public static BllobContentId FromHash(ReadOnlySpan<byte> hashCode);
}

public struct BlobReader
{
    public void ReadBytes(Span<byte> buffer);
    public void ReadUTF8(int byteCount, Span<char> buffer);
    public bool TryReadUTF8(int byteCount, Span<char> buffer, out int charsWritten);
    public void ReadUTF16(int byteCount, Span<char> buffer);
    public bool TryReadUTF16(int byteCount, Span<char> buffer, out int charsWritten);
}

public struct BlobWriter
{
    public void WriteBytes(ReadOnlySpan<byte> buffer);
}

public struct MetadataReader
{
    public int GetBlobLength(BlobHandle handle);
    public void GetBlobBytes(BlobHandle handle, Span<byte> buffer, out int bytesWritten);
    public int GetStringLength(StringHandle handle);
    public void GetStringBytes(StringHandle handle, Span<char> buffer, out int bytesWritten);
    public int GetStringLength(NamespaceDefinitionHandle handle);
    public void GetStringBytes(NamespaceDefinitionHandle handle, Span<char> buffer, out int bytesWritten);
    public int GetStringLength(DocumentNameBlobHandle handle);
    public void GetStringBytes(DocumentNameBlobHandle handle, Span<char> buffer, out int bytesWritten);
    public int GetUserStringLength(UserStringHandle handle);
    public void GetUserStringBytes(UserStringHandle handle, Span<char> buffer, out int bytesWritten);

    // Or what about we add the following APIs instead of the above?
    // The memory the span points to can be freed under our feet but
    // this is an existing problem with many SRM APIs that safely wrap pointers.
    // The library is expert-level either way.
    public ReadOnlySpan<byte> GetBlobSpan(BlobHandle handle);
    public ReadOnlySpan<byte> GetRawStringBytes(StringHandle handle);
    public ReadOnlySpan<byte> GetRawStringBytes(NamespaceDefinitionHandle handle);
    public ReadOnlySpan<byte> GetRawStringBytes(DocumentNameBlobHandle handle);
    public ReadOnlySpan<byte> GetRawUserStringBytes(UserStringHandle handle);
}

namespace System.Reflection.Metadata.Ecma335;

public sealed class MetadataBuilder
{
    // These APIs will not allocate if the blob/string already exists.
    public BlobHandle GetOrAddBlob(ReadOnlySpan<byte> value);
    public BlobHandle GetOrAddBlobUTF8(ReadOnlySpan<byte> value);
    public BlobHandle GetOrAddBlobUTF16(ReadOnlySpan<byte> value);
    public BlobHandle GetOrAddDocumentName(ReadOnlySpan<char> value);
    public StringHandle GetOrAddString(ReadOnlySpan<char> value);
    public UserStringHandle GetOrAddUserString(ReadOnlySpan<char> value);
}

API Usage

The proposed APIs correspond to existing APIs that work with strings and (immutable) byte arrays. Their usage will be similar.

Alternative Designs

Do nothing and either let users implement the span APIs on top of pointers if they are available, or accept the memory allocations if they aren't.

There are also more possible APIs to be spanified (such as methods in the System.Reflection.Metadata.Ecma335.***Encoders), but the proposed APIs are the most foundational; the rest can be implemented by user code on top of these with little effort.

Risks

No response

Author:	teo-tsirpanis
Assignees:	-
Labels:	api-suggestion, area-System.Reflection.Metadata
Milestone:	-

[steveharter]

I propose additional span overloads for methods that work with memory buffers

@teo-tsirpanis can you provider an example of the caller using buffers (not backed by a simple array) where these APIs are advantageous? Thanks

[teo-tsirpanis]

One use case would be #84580 (comment).

[buyaa-n]

@teo-tsirpanis the API proposals for BlobBuilder, BlobContentId, BlobWriter and MetadataBuilder looks good to me, for others more info needed, I left some questions as a comment. In general, a usage scenarios for them would be helpful for review. Also, I would prefer to split the proposals for BlobReader and MetadataReader into another issue if that makes sense to you

public struct BlobReader
{
    public byte[] ReadBytes(int byteCount)
    public void ReadBytes(int byteCount, byte[] buffer, int bufferOffset)
+   public void ReadBytes(Span<byte> buffer);
    public string ReadUTF8(int byteCount)
+   public void ReadUTF8(int byteCount, Span<char> buffer); // Why this passes byteCount but ReadBytes(Span<byte> buffer) not?
+   public bool TryReadUTF8(int byteCount, Span<char> buffer, out int charsWritten); // A uage scenarios will be helpful
    public string ReadUTF16(int byteCount)
+   public void ReadUTF16(int byteCount, Span<char> buffer);
+   public bool TryReadUTF16(int byteCount, Span<char> buffer, out int charsWritten); // Usage scenarios will be helpful
}

public struct MetadataReader
{
+   public int GetBlobLength(BlobHandle handle); // Why this needed? I guess for determining span size, anyway a user scenario will be useful
    public byte[] GetBlobBytes(BlobHandle handle);
+   public void GetBlobBytes(BlobHandle handle, Span<byte> buffer, out int bytesWritten); // why do you need bytesWritten?
    public string GetString(StringHandle handle);
+   public int GetStringLength(StringHandle handle); // usage scenarios
+   public void GetStringBytes(StringHandle handle, Span<char> buffer, out int bytesWritten);
+   public int GetStringLength(NamespaceDefinitionHandle handle);
+   public void GetStringBytes(NamespaceDefinitionHandle handle, Span<char> buffer, out int bytesWritten);
+   public int GetStringLength(DocumentNameBlobHandle handle);
+   public void GetStringBytes(DocumentNameBlobHandle handle, Span<char> buffer, out int bytesWritten);
+   public int GetUserStringLength(UserStringHandle handle);
+   public void GetUserStringBytes(UserStringHandle handle, Span<char> buffer, out int bytesWritten);

    // Or what about we add the following APIs instead of the above?
    // The memory the span points to can be freed under our feet but
    // this is an existing problem with many SRM APIs that safely wrap pointers.
    // The library is expert-level either way.
+    public ReadOnlySpan<byte> GetBlobSpan(BlobHandle handle);
+    public ReadOnlySpan<byte> GetRawStringBytes(StringHandle handle);
+    public ReadOnlySpan<byte> GetRawStringBytes(NamespaceDefinitionHandle handle);
+    public ReadOnlySpan<byte> GetRawStringBytes(DocumentNameBlobHandle handle);
+    public ReadOnlySpan<byte> GetRawUserStringBytes(UserStringHandle handle);
}

public sealed class MetadataBuilder
{
    public BlobHandle GetOrAddBlob(byte[] value) 
    // These APIs will not allocate if the blob/string already exists.
+   public BlobHandle GetOrAddBlob(ReadOnlySpan<byte> value);
    public BlobHandle GetOrAddBlobUTF8(string value, bool allowUnpairedSurrogates = true);
+   public BlobHandle GetOrAddBlobUTF8(ReadOnlySpan<byte> value); // should it have allowUnpairedSurrogates parameter too?
    public System.Reflection.Metadata.BlobHandle GetOrAddBlobUTF16(string value);
+   public BlobHandle GetOrAddBlobUTF16(ReadOnlySpan<byte> value);
    public BlobHandle GetOrAddDocumentName(string value);
+   public BlobHandle GetOrAddDocumentName(ReadOnlySpan<char> value);
    public StringHandle GetOrAddString(string value);
+   public StringHandle GetOrAddString(ReadOnlySpan<char> value);
    public UserStringHandle GetOrAddUserString(string value);
+   public UserStringHandle GetOrAddUserString(ReadOnlySpan<char> value);
}

Adding future milestone for now, I don't think all of these APIs ready for 8.0, we could change milestone if BlobReader and MetadataReader proposals moved to an another issue/proposal

[fowl2]

Missing the simplest one :)

public sealed class MetadataReader
{
    public unsafe byte* MetadataPointer { get; }
    public int MetadataLength { get; }
+   public ReadOnlySpan<byte> MetadataSpan { get; }
}

Not sure if this is that useful given the whole type is unsafe, but perhaps:

public unsafe struct BlobReader
{
+   public BlobReader(ReadOnlySpan<byte> buffer);
}

[PaulusParssinen]

ILCompiler and its components could benefit from more (RO)Span<T> overloads in the S.R.M. In many places ILC has to go through various hoops to cache values that it reads from metadata because lack of modern Span APIs.

[teo-tsirpanis]

@buyaa-n

public int GetBlobLength(BlobHandle handle); // Why this needed? I guess for determining span size, anyway a user scenario will be useful

My thought was that it would allow the users to get the buffer's size and prepare an appropriately sized buffer to pass to GetBlob.

public void ReadUTF8(int byteCount, Span<char> buffer); // Why this passes byteCount but ReadBytes(Span<byte> buffer) not?
public bool TryReadUTF8(int byteCount, Span<char> buffer, out int charsWritten); // A uage scenarios will be helpful

On second thought byteCount is not needed; it can be inferred from the span. I updated the proposal to fix this and other mistakes. I also am not sure if the BlobReader.TryRead*** methods are needed.

[AArnott]

public void GetStringChars(NamespaceDefinitionHandle handle, Span<char> buffer, out int bytesWritten);

The out parameter should be named charsWritten instead. For this, and all other char-based spans with out length parameters.