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

Static ReadOnlySpan<byte> optimization reduces memory allocation #1044

Merged

Conversation

pCYSl5EDgo
Copy link
Contributor

private static readonly byte[] Hoge = new byte[] { /* some values */ };

can be optimized by being replaced with

private static ReadOnlySpan<byte> GetHoge() => new byte[] { /* some values */ };

Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. TIL 🎉. Thanks.

@AArnott
Copy link
Collaborator

AArnott commented Sep 15, 2020

I had to check sharplab to make sure we weren't allocating arrays on every invocation. Sure enough, you're right. :)

@AArnott AArnott merged commit 2859cdf into MessagePack-CSharp:v2.2 Sep 15, 2020
@pCYSl5EDgo pCYSl5EDgo deleted the StaticByteArrayOptimization branch September 15, 2020 15:28
@neuecc
Copy link
Member

neuecc commented Sep 15, 2020

@AArnott @pCYSl5EDgo
Is this efficient in Unity and IL2CPP?
Basically, I believe it will work with the Slow Span equivalent.
It seems to me that the savings in .NET Core are not balanced by the penalties in IL2CPP.

@AArnott
Copy link
Collaborator

AArnott commented Sep 15, 2020

Oooh. Good point, @neuecc. Shall we roll it back?

It's easy enough to see how the C# compiler interprets this and verify that it's a win. How can we tell what the Unity C# compiler does?
If IL2CPP takes IL and converts it to native, I would guess it doesn't make it worse. But if it takes the Unity C# compiler's IL as input, then we might still have a problem.

@pCYSl5EDgo
Copy link
Contributor Author

I'll install Unity2018.4.27f1 and emit IL2CPP C++ codes.
Unity 2018.x uses csc v2.9.1.x.
I guess IL generated by Unity C# compiler is not different from the one of .NET Core.

Anyway, I'll examine it.

@neuecc
Copy link
Member

neuecc commented Sep 16, 2020

IL is the same, but JIT's optimization for Span doesn't work.
In other words, new ReadOnlySpan..., indexer access to Span is slower than .NET Core.
It's probably not going to make much of a difference in performance.
However, I think should not do minor tricks if it's not a very beneficial in .NET Core.

I think we'll leave it as it is this time, but it's one of the factors we'll consider in the future.

@pCYSl5EDgo
Copy link
Contributor Author

Okay, it is not the time.
We should roll it back.

I think the time is when the oldest Unity LTS supports .NET 5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants