Skip to content
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

[API Proposal]: ReadOnlySpan<T> CreateSpan<T>(RuntimeFieldHandle) #60948

Closed
davidwrighton opened this issue Oct 27, 2021 · 34 comments
Closed

[API Proposal]: ReadOnlySpan<T> CreateSpan<T>(RuntimeFieldHandle) #60948

davidwrighton opened this issue Oct 27, 2021 · 34 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Memory
Milestone

Comments

@davidwrighton
Copy link
Member

davidwrighton commented Oct 27, 2021

Background and motivation

As of C# 8.0 it is possible to embed constant byte data into .NET Assemblies using a syntax like

ReadOnlySpan<byte> someData = new byte[] { 0, 1 };

However, unlike the support for array initialization this isn't possible for other primitive data types. Instead for other data types the ReadOnlySpan is initialized by actually allocating an array. The reason for this lack of support is that there is no current way to express that the constant data is in little endian format, and needs to be translated to the runtime endian format, if the application is run on hardware which utilizes big endian numbers.

This proposal is designed to provide a new api which works in a manner similar to how array initialization via RuntimeHelpers.InitializeArray works. This will allow constant data to be cleanly expressed on all architectures.

During a recent hackathon event, support for this was prototyped, and a simple example such as

    static int[] _intData = new int[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int GetData(int offset)
    {
        return _intData[offset];
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int GetDataROS(int offset)
    {
        ReadOnlySpan<int> intSpan = (ReadOnlySpan<int>)new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
        return intSpan[offset];
    }

was shown to be a fair bit faster.

In particular, looped 1.8 billion times GetData took 9783ms and 'GetDataROS' took 8468ms to execute. For an improvement of somewhere around 13.5%.

@jaredpar has looked at the hackathon effort and is in general agreement that the C# team would be willing to implement code generation of this.

@stephentoub has also done some investigation of locations within the BCL where this sort of data would be useful, and there are a significant number of potential optimizations. Individually, these optimizations are generally fairly small, but there are quite a few which might benefit.

API Proposal

namespace System.Runtime.CompilerServices
{
    public static class RuntimeHelpers
    {
        public ReadOnlySpan<T> CreateSpan<T>(RuntimeFieldHandle fldHandle);
    }
}

Requirements:
T must be a primitive constant sized type (byte, sbyte, char, short, ushort, int, uint, long, ulong, float, double)
field must reference a FieldRVA
The RVA associated with field must be aligned on the size of the primitive type T.

API Usage

ReadOnlySpan<int> intSpan = new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

Or possibly if the C# team wish to implement...

ReadOnlySpan<int> intSpan = stackalloc int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

There is no expectation that developers not generating IL would ever use this api directly.

Alternative Designs

It may be simpler for the JIT to work with the following construct. However, I don't believe it is a significant improvement.

namespace System.Runtime.CompilerServices
{
    public static class RuntimeHelpers
    {
        public CreateSpan<T>(RuntimeFieldHandle field, out ReadOnlySpan<T> span);
    }
}

Risks

Since the new api would only be available on new versions of .NET but the syntax is valid for older frameworks, customers writing code may write code expecting the optimized behavior and instead see substantially slower, allocating logic instead. Suggestions have been made that support for this may also entail building a fallback generator for this sort of thing when targeting older versions of the frameworks to avoid performance cliffs.

@davidwrighton davidwrighton added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 27, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Memory untriaged New issue has not been triaged by the area owner labels Oct 27, 2021
@ghost
Copy link

ghost commented Oct 27, 2021

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

Issue Details

Background and motivation

As of C# 8.0 it is possible to embed constant byte data into .NET Assemblies using a syntax like

ReadOnlySpan<byte> someData = new byte[] { 0, 1 };

However, unlike the support for array initialization this isn't possible for other primitive data types. Instead for other data types the ReadOnlySpan is initialized by actually allocating an array. The reason for this lack of support is that there is no current way to express that the constant data is in little endian format, and needs to be translated to the runtime endian format, if the application is run on hardware which utilizes big endian numbers.

This proposal is designed to provide a new api which works in a manner similar to how array initialization via RuntimeHelpers.InitializeArray works. This will allow constant data to be cleanly expressed on all architectures.

During a recent hackathon event, support for this was prototyped, and a simple example such as

    static int[] _intData = new int[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int GetData(int offset)
    {
        return _intData[offset];
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int GetDataROS(int offset)
    {
        ReadOnlySpan<int> intSpan = (ReadOnlySpan<int>)new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
        return intSpan[offset];
    }

was shown to be a fair bit faster.

In particular, looped 1.8 billion times GetData took 9783ms and 'GetDataROS' took 8468ms to execute. For an improvement of somewhere around 13.5%.

@jaredpar has looked at the hackathon effort and is in general agreement that the C# team would be willing to implement code generation of this.

@stephentoub has also done some investigation of locations within the BCL where this sort of data would be useful, and there are a significant number of potential optimizations. Individually, these optimizations are generally fairly small, but there are quite a few which might benefit.

API Proposal

namespace System.Runtime.CompilerServices
{
    public static class RuntimeHelpers
    {
        public ReadOnlySpan<T> CreateSpan<T>(RuntimeFieldHandle field);
    }
}

Requirements:
T must be a primitive constant sized type (byte, sbyte, char, short, ushort, int, uint, long, ulong, float, double)
field must reference a FieldRVA
The RVA associated with field must be aligned on the size of the primitive type T.

API Usage

ReadOnlySpan<int> intSpan = new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

Or possibly if the C# team wish to implement...

ReadOnlySpan<int> intSpan = stackalloc int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

There is no expectation that developers not generating IL would ever use this api directly.

Alternative Designs

It may be simpler for the JIT to work with the following construct. However, I don't believe it is a significant improvement.

namespace System.Runtime.CompilerServices
{
    public static class RuntimeHelpers
    {
        public CreateSpan<T>(RuntimeFieldHandle field, out ReadOnlySpan<T> span);
    }
}

Risks

Since the new api would only be available on new versions of .NET but the syntax is valid for older frameworks, customers writing code may write code expecting the optimized behavior and instead see substantially slower, allocating logic instead. Suggestions have been made that support for this may also entail building a fallback generator for this sort of thing when targeting older versions of the frameworks to avoid performance cliffs.

Author: davidwrighton
Assignees: -
Labels:

api-suggestion, area-System.Memory, untriaged

Milestone: -

@davidwrighton
Copy link
Member Author

@AaronRobinsonMSFT (Who worked on the hackathon on this project)
@svick
@JulieLeeMSFT This requires a new JIT intrinsic, could you suggest an engineer to review the intrinsic and provide an opinion about this?

@stephentoub
Copy link
Member

Nice write-up. #24961 (comment) was already approved... is this different?

@jkotas
Copy link
Member

jkotas commented Oct 27, 2021

Nit: The approved API has fldHandle as the argument name, but it is exactly the same otherwise.

@JulieLeeMSFT
Copy link
Member

JulieLeeMSFT commented Oct 27, 2021

@echesakovMSFT PTAL.
cc @tannergooding

Nit: The approved API has fldHandle as the argument name, but it is exactly the same otherwise.

@jkotas if it is the exactly same API with a different argument name, should we close this one and merge this thread with #24961?

@jkotas
Copy link
Member

jkotas commented Oct 27, 2021

Let's keep this one open since it follows the API proposal template and close #24961.

@jkotas jkotas added api-approved API was approved in API review, it can be implemented and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Oct 27, 2021
@VSadov
Copy link
Member

VSadov commented Oct 28, 2021

There is a difference between the two proposed use variants. I think Roslyn should implement optimization for both.

Re:

API Usage

ReadOnlySpan<int> intSpan = new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

Or possibly if the C# team wish to implement...

ReadOnlySpan<int> intSpan = stackalloc int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

When optimized, these will result in the same IL, but language treats them differently.

Span local will assume escape scope from its initializer and stackalloc'd spans cannot escape the method scope.
On the other hand new int[] variant has global scope - i.e. can be returned from a method, so user would typically use that to have a less constrained variable.

{
     ReadOnlySpan<int> intSpan = new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
     // this is ok
     return intSpan;
}

However there is a scenario where user would want the more restricted stackalloc int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; variant.
That is when escaping the span is not the goal, but reinitializing the span with another stackalloc is a possibility.

Ex:

ReadOnlySpan<int> intSpan = stackalloc int[]{1, 2, 3};

if (condition)
{
    // if intSpan is globally escapable, this would be a compile error
    intSpan = stackalloc int[] {x, y, z,};
}

stackalloc does not produce garbage, but no-copy initialization is still cheaper, so stackalloc variant should be optimized too.

@davidwrighton
Copy link
Member Author

@VSadov I agree that the various C# lowerings/optimizations are interesting to discuss, but I don't believe that this is the right forum. I believe the discussion about the precise lowering, and when it should happen, etc should be done in the Roslyn or C# language repos.

@VSadov
Copy link
Member

VSadov commented Oct 28, 2021

Right, When corresponding Roslyn workitem is created, this should go there. I do not think we have one yet.

@teo-tsirpanis
Copy link
Contributor

How about implementing ReadOnlySpan literals of larger-than-byte types by storing both the little-endian and the big-endian byte representation and deciding at runtime? The following example:

ReadOnlySpan<int> mySpan = new int[] {475, 579};

would be translated to something like this:

ReadOnlySpan<int> mySpan =
    new ReadOnlySpan<int>(BitConverter.IsLittleEndian ? &mySpan_littleEndian : &mySpan_bigEndian, 2);

The advantages are performance (the IsLittleEndian will most likely be optimized away and the bounds check eliminator would learn more easily the span's length) and no need for runtime support or new APIs. The disadvantage is that such literals will take double the space on the assembly, but it can be easily solved with trimming (and how much of these literals would an assembly hold to make this a problem?).

@jaredpar
Copy link
Member

T must be a primitive constant sized type (byte, sbyte, char, short, ushort, int, uint, long, ulong, float, double)

Why this fixed set of types vs. just all types that are unmanaged? Essentially why not do where T : unmanaged.

public ReadOnlySpan CreateSpan(RuntimeFieldHandle fldHandle);

Nit: all of these need to include static

Since the new api would only be available on new versions of .NET but the syntax is valid for older frameworks, customers writing code may write code expecting the optimized behavior and instead see substantially slower, allocating logic instead. Suggestions have been made that support for this may also entail building a fallback generator for this sort of thing when targeting older versions of the frameworks to avoid performance cliffs.

I wanted to push back on this a bit by using the following code as an example:

ReadOnlySpan<byte> span = new byte { 0x1, 0x2, 0x3 }; 

This code is not universally optimized, it only optimizes on certain versions of the compiler. Yet we never have gotten any push back about that. Instead the general feeling is if you want the faster optimization then upgrade to the faster toolset + runtime.

@stephentoub
Copy link
Member

This code is not universally optimized, it only optimizes on certain versions of the compiler. Yet we never have gotten any push back about that. Instead the general feeling is if you want the faster optimization then upgrade to the faster toolset + runtime.

We haven't gotten pushback I expect because you don't need a newer set of reference assemblies to use this, all you need is the newer compiler. We're able to rely on this without question anywhere in dotnet/runtime because we know we're using a compiler that provides the optimization, even if we're building for netstandard2.0. Remove that guarantee, and we now need to think about every place we make this change, as the file we're modifying might be built into an assembly targeting an older set of reference assemblies.

@jkotas
Copy link
Member

jkotas commented Oct 29, 2021

Why this fixed set of types vs. just all types that are unmanaged? Essentially why not do where T : unmanaged.

The complexity for that goes through the roof. It was explored in dotnet/designs#46.

@tannergooding
Copy link
Member

The complexity for that goes through the roof

It's not clear to me why this is the case. For a little-endian machine this should be a direct read of the data, iterated per field.

For a big endian machine, we need to:

  • Allocate Memory
    • We have multiple options here including RuntimeHelpers.AllocateTypeAssociatedMemory, the PinnedObjectHeap, NativeMemory.Alloc, etc.
  • For each field/element of the target type, do effectively System.Buffers.Binary.BinaryPrimitives.ReverseEndianness

This is a very simple thing, algorithmically speaking. It does have some up-front cost for big-endian machines but RyuJIT doesn't actually support that today and for Mono its a rare target.

===========

The IL spec already covers everything required here in II.16.3 Embedding data in a PE file.

The IL grammar itself allows for structured data declarations:

DataDecl ::= [ DataLabel ‘=’ ] DdBody

DdBody ::= DdItem 
         | ‘{’ DdItemList ‘}’

DdItemList ::= DdItem [ ‘,’ DdItemList ]

DdItem ::= ‘&’ ‘(’ Id ‘)’
         | bytearray ‘(’ Bytes ‘)’
         | char ‘*’ ‘(’ QSTRING ‘)’
         | float32 [ ‘(’ Float64 ‘)’ ] [ ‘[’ Int32 ‘]’ ]
         | float64 [ ‘(’ Float64 ‘)’ ] [ ‘[’ Int32 ‘]’ ]
         | int8 [ ‘(’ Int32 ‘)’ ] [‘[’ Int32 ‘]’ ]
         | int16 [ ‘(’ Int32 ‘)’ ] [ ‘[’ Int32 ‘]’ ]
         | int32 [ ‘(’ Int32 ‘)’ ] [‘[’ Int32 ‘]’ ]
         | int64 [ ‘(’ Int64 ‘)’ ] [ ‘[’ Int32 ‘]’ ]

and so something such as the following is legal:

    .data OneData = {
        float32 (1.0),
        float32 (1.0),
        float32 (1.0),
        float32 (1.0)
    }

    .field public static initonly Vector4 One = at OneData

It likewise covers the 3 types of data initialization, all of which are supported on all target platforms either via PE or the ELF files for AOT scenarios or via the runtime itself for JIT scenarios:

  • Data known at link time
  • Data known at load time
  • Data known at run time

The first case is the primitive case where everything is well-known ahead of time.
The second case covers scenarios like ‘&’ ‘(’ Id ‘)’ where you have inter declaration references
The third case covers the big endian scenario, which can be covered via one of the many existing initialization mechanisms we support (type initializers, module initializers, extending the PE/ELF loader logic, etc)

@jkotas
Copy link
Member

jkotas commented Oct 30, 2021

For a big endian machine, we need to:

You need to do it for little endian machines too to deal with platform specific alignment rules.

@tannergooding
Copy link
Member

You need to do it for little endian machines too to deal with platform specific alignment rules.

In some cases involving badly packed layouts, yes. There are just as many scenarios, particularly involving our own simple value types that are all laid out in a way that's the same across all platforms.

  • Basically anything that doesn't involve custom packing or pointers (variable sized types) has a consistent layout across all platforms
  • There is also the odd exception for double, long, or ulong on 32-bit Unix, but that's likewise going to be fairly rare
  • Things like Guid or Vector2/3/4, Quaternion, Complex, Color, etc are the same on all platforms/architectures
  • Many native structs already try to take "natural packing" into account to save space; out of the some 24k bindings I maintain for popular native libraries across Windows and Unix; there are only a handful that are special here

I'm still confident this is something that could be handled by our tooling and the benefits it could provide could be substantial, particularly in interop, graphics, and HPC related scenarios (the same goes for constexpr like support if we eventually got that).


I'm already working around this in a few of my interop libraries. I have nearly 7000 "static readonly" variables that have to be initialized in one library. These were almost all Guids and that was adding significant startup cost.

Changing it all to do this, brought the startup cost to 0 (big endian isn't supported here, but my generator inserts the right stuff if I tell it to):

[NativeTypeName("const GUID")]
public static ref readonly Guid IID_ID3D12Device
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    get
    {
        ReadOnlySpan<byte> data = new byte[] {
            0xF1, 0x19, 0x98, 0x18,
            0xB6, 0x1D,
            0x57, 0x4B,
            0xBE,
            0x54,
            0x18,
            0x21,
            0x33,
            0x9B,
            0x85,
            0xF7
        };

        Debug.Assert(data.Length == Unsafe.SizeOf<Guid>());
        return ref Unsafe.As<byte, Guid>(ref MemoryMarshal.GetReference(data));
    }
}

@jkotas
Copy link
Member

jkotas commented Oct 30, 2021

I agree that the support for arbitrary types is implementable. It just makes the feature multiple times more complex and expensive to implement. The proposal for primitive types is compatible with eventual extension for arbitrary types if we choose to do it. I think it makes sense to start with primitive types only.

The Guid example from your library does not look like a good motivating scenario for support of arbitrary types. The main problem is that you have all thousands of Guids in one type. Nothing good performance-wise comes from types with thousands of fields or methods. The cost for loading types like that is far from zero. This hack above helps you eliminate the JIT cost of the huge static constructor, but you are still going to pay for the thousands fields or methods even if you are using just a few of them. It think a better factoring would be to have the IID as part of the containing type or something like that. For example, how it is done in CsWinRT: https://github.com/microsoft/CsWinRT/blob/72b1236979de2d444416207c5d14bc513af52592/src/WinRT.Runtime/Interop/IWeakReferenceSource.net5.cs#L80 . Once you do that, the startup overhead of naturally initialized Guid constants becomes much more pay-for-play. For scenarios that care a lot about startup performance, a good AOT compiler should be able to run the initialization for naturally initialized Guid at compile time, without any additional measures.

@tannergooding
Copy link
Member

The proposal for primitive types is compatible with eventual extension for arbitrary types if we choose to do it. I think it makes sense to start with primitive types only.

I think that makes sense, but at the same time if we are going to do something here it'd be nice to finish it all out in the same release. Getting partway there for .NET 7 and then having to wait an entire separate release (or more) to get the rest can be painful.

The main problem is that you have all thousands of Guids in one type

There are multiple reasons for this, including (but not limited to) backwards/forwards compatibility and ease of use/porting native code to C#. I did, a long time ago, actually have many separate classes, assemblies, and "clearer" separation of types (such as SomeType.IID). However, across multiple releases of a given native library and due to weird interdependencies that can exist between C/C++ headers, this quickly broke down. Likewise, putting things in many separate types forces users to remember where everything is and to hinders their ability to follow existing C/C++ samples or other logic that exists.

Putting all the "globally visible" C/C++ members into a single type has helped solve many more problems than it has caused so far, especially given my assemblies are fully trimmable (and so shrink from 12MB to about 50-200kb for most usages).

Namely, it means that my libraries are "as close to #include <Windows.h> as you can get in .NET" (or respective other header for other libraries, like Vulkan.h, Xlib.h, PulseAudio, etc). All you need to do is reference the package and add using TerraFX.Interop; plus using static TerraFX.Interop.Windows; (or respective other class name, like Vulkan). You can then trivially take a large portion of existing C/C++ samples and get them to work nearly 1-to-1 in C#. Same names, same casing, same general visibility, etc.

The largest downside was, by far, the cost of the static initializers; which was mitigated by moving to a mix of properties and "unmanaged constants". There is still some cost for the VM to load and process the main class, but it was basically immeasurable in comparison (its also no worse than what C++/CLI does for importing metadata; which while not the perfect example, is an apt-comparison of something you can already hit for larger interop libraries).

@iSazonov
Copy link
Contributor

iSazonov commented Nov 3, 2021

I'm glad to see a progress for primitive type constants. Is there something similar for more complex types? So, PowerShell builds some large Dictionary-based caches on startup. It would be great if they could be built beforehand at the crossgen stage, for example.

@jkotas
Copy link
Member

jkotas commented Nov 3, 2021

@iSazonov Can you link an example of the large Dictionary cache in Powershell? (To me, it sounds like a job for source generator and specialized hashtable structure optimized for persistence.)

@iSazonov
Copy link
Contributor

iSazonov commented Nov 3, 2021

@jkotas There are two examples.

  1. A type catalog. The cs file is generated (by custom application for years since we get source generators only now) so I attach the result file
    CorePsTypeCatalog.zip

  2. It is more complex example. Historically Windows PowerShell loads psxml files at startup. You can find them in C:\Windows\System32\WindowsPowerShell\v1.0 folder (ex., types.ps1xml). Now PowerShell Core has the files converted to cs code that excludes XML parsing but we still have to build Dictionary at startup. See https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/TypeTable_Types_Ps1Xml.cs

With .Net 6.0 I see great improvement of PowerShell startup scenario (~20% on my note. Thanks .Net team for great work!) (Nevertheless, it is still a long way from Windows PowerShell - 400 ms vs 270 ms on my note.)
As result now I see in PerfView traces many issues with startup data initialization :-) (in both PowerShell and .Net)
In addition to the examples I gave, it is also static initializers. (And these are not always constants that is discussed in that issue)
No doubt it would be great to pre-compile these data in some way.

jbevain pushed a commit to jbevain/cecil that referenced this issue Jan 19, 2022
* FieldRVA alignment

In support of dotnet/runtime#60948 the linker (an assembly rewriter) will need to be able to preserve the alignment of RVA based fields which are to be used to create the data for `CreateSpan<T>` records

This is implemented by adding a concept that RVA fields detect their required alignment by examining the PackingSize of the type of the field (if the field type is defined locally in the module)

* Update Mono.Cecil.Metadata/Buffers.cs

Co-authored-by: Aaron Robinson <arobins@microsoft.com>

* Enhace logic used to ensure type providing PackingSize is local to the module.

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
davidwrighton added a commit that referenced this issue Jan 27, 2022
In support of CreateSpan (#60948), improve alignment for RVA static pre-initialized fields to align memory blocks which may contain long, ulong, or double primitive arrays on 8 byte boundaries.

Mono fix is more involved
- Fix Ref-Emit generated RVA statics
 - Set the HasFieldRVA bit. NOTE: earlier code that attempts to set the bit doesn't actually do that as the FieldRVA bit is in  FieldAttributes.ReservedMask which is masked away in the FieldBuilder constructor
 - Fix the Swizzle lookup. We should not be using the 8 byte swizzle in the final else clause.
- Enhance ref-emitted field to allow for use with CreateSpan
  - Ref-emitted fields should specify a pack size appropriate for minimum alignment of the CreateSpan targetted data
  - Respect the packing_size specified so that RVA static fields generated for CreateSpan can be properly aligned

Fixes #62314
@stephentoub
Copy link
Member

@davidwrighton, is there work remaining on this issue, or is the API functional enough now for Roslyn to target?

@echesakov
Copy link
Contributor

Un-assigning myself
cc @BruceForstall

@echesakov echesakov removed their assignment Mar 15, 2022
@davidwrighton
Copy link
Member Author

There was 1 open question from @MichalStrehovsky as to whether or not we should mandate the explicit higher alignment via the .pack directive in CreateSpan, or rely on the compilers to do this without additional validation, but its ready for Roslyn to target.

@stephentoub
Copy link
Member

Excellent.

@GSPP
Copy link

GSPP commented May 23, 2022

Why is it called fldHandle and not fieldHandle? Is this a convention for such APIs? Such abbreviations are rarely seen in the framework.

@MichalStrehovsky
Copy link
Member

Why is it called fldHandle and not fieldHandle? Is this a convention for such APIs? Such abbreviations are rarely seen in the framework.

Consistency with RuntimeHelpers.InitializeArray I guess. It's not expected for developers to call this API from C#.

@GSPP
Copy link

GSPP commented May 24, 2022

If there are no expected managed callers anyway then I'd fix the names on both methods.

@stephentoub
Copy link
Member

I'd fix the names on both methods.

There's practically zero benefit.

@stephentoub
Copy link
Member

Just as status:
#70179 rolls out use of the feature in dotnet/runtime, but it's blocked on the compiler using the new API. dotnet/roslyn#61414 teaches the compiler to use the new API, but it's blocked on a versioning issue with VS and System.Reflection.Metadata that @jaredpar is working on.

@jeffhandley jeffhandley added this to the 7.0.0 milestone Jul 10, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 27, 2022
@stephentoub stephentoub modified the milestones: 7.0.0, 8.0.0 Jul 27, 2022
@jkotas jkotas mentioned this issue Sep 23, 2022
29 tasks
@stephentoub
Copy link
Member

dotnet/roslyn#61414 was merged, so I'm going to consider this done. I have a separate commit from dotnet/runtime to switch over a bunch of arrays to spans once we ingest an updated compiler that has the addition. Thanks, all.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 5, 2023
@stephentoub
Copy link
Member

dotnet/runtime is now consuming the Roslyn build that has support for the previously added CreateSpan method, and code in dotnet/runtime is now taking advantage of it, so this work is all done. Thanks to all who contributed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Memory
Projects
None yet
Development

Successfully merging a pull request may close this issue.