Skip to content

Implement internal ISpanFormattable on {U}IntPtr #44496

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.

Sign up for GitHub

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

Jump to bottom
Merged
jkotas merged 1 commit into from
Nov 11, 2020
Merged

Implement internal ISpanFormattable on {U}IntPtr #44496

jkotas merged 1 commit into from
Nov 11, 2020

Conversation

@stephentoub
Copy link
Member

@stephentoub stephentoub commented Nov 10, 2020

Allows nint and nuint to participate in the same optimizations in string building / formatting as do the other primitives.

cc: @tannergooding

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Diagnosers;
using BenchmarkDotNet.Running;

[MemoryDiagnoser]
public class Program
{
    static void Main(string[] args) => BenchmarkSwitcher.FromAssemblies(new[] { typeof(Program).Assembly }).Run(args);

    private nint _value = 123456789;

    [Benchmark]
    public string Format() => $"Value: {_value}";
}
Method Job Toolchain Mean Ratio Allocated
Format Job-RUVFJN \master\corerun.exe 81.18 ns 1.00 120 B
Format Job-IDOMKL \pr\corerun.exe 68.24 ns 0.84 80 B

(Separately, we might want to expose these TryFormat methods publicly.)

@stephentoub
Implement internal ISpanFormattable on {U}IntPtr
e4f13bb 
Allows nint and nuint to participate in the same optimizations in string building / formatting as do the other primitives.
@stephentoub stephentoub added this to the 6.0.0 milestone Nov 10, 2020
@tannergooding
Copy link
Member

tannergooding commented Nov 10, 2020

This LGTM. However, do we need to take this to API review just for consistencies sake (even if just a quick over e-mail confirmation)?

@stephentoub
Copy link
Member Author

stephentoub commented Nov 10, 2020 •
edited
Loading

However, do we need to take this to API review just for consistencies sake (even if just a quick over e-mail confirmation)?

These are internal.

But we can open an issue about exposing them.

@tannergooding
Copy link
Member

tannergooding commented Nov 10, 2020

Ah right, I forgot ISpanFormattable was internal. 👍

@jkotas jkotas merged commit 0efefa8 into dotnet:master Nov 11, 2020
@stephentoub stephentoub deleted the intptrformattable branch November 11, 2020 03:41
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas approved these changes

@tannergooding tannergooding tannergooding approved these changes

Assignees

No one assigned

Labels

area-System.Runtime tenet-performance Performance related issue

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@stephentoub @tannergooding @jkotas