-
Notifications
You must be signed in to change notification settings - Fork 48
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
Aggressive optimization and inlining foor PooledList.Span #34
Comments
No optimization tag:
|
Span marked with aggressive inlining:
|
So:
|
Sorry for the delayed response. Here's what I'm currently seeing without your suggested optimization, run from the "core3" branch:
And here's what I'm seeing with AggressiveInlining applied to the "Span" property getter. There doesn't seem to be a significant change by doing this. Perhaps I'm misunderstanding what you're proposing?
public Span<T> Span
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get
{
return _items.AsSpan(0, _size);
}
} |
MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization ? It may not help either. But at least, in Unity IL2CPP both span and index were slow, so people put items array as public. So if nothing changes, may be leave both optimization in place so that possible other runtimes will have more chances to hit the optimization? Just as idea, but anyway - may close the issue. Thanks |
Is your feature request related to a problem? Please describe.
Next method is kind of slow in Unity runtime (not sure about Core):
Describe the solution you'd like
Mark this method as aggressive optimization and inlining.
Describe alternatives you've considered
Provide unsafe access to writable array as ReadOnlyProperty.
Additional context
We get data from Span by ref.
I know Core 3 optimized Span, but there are issues to measure that because of BDN-JIT instability. Same time Unity is involved into .NET Ecosystem and hopefully will optimize Span in some time (because people will use .NET Standard with Spans and Unity stated it is 2.0 and will be 2.1)
The text was updated successfully, but these errors were encountered: