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

R2R image of SPC has many costly ValueTuple methods #76513

Open
jakobbotsch opened this issue Oct 2, 2022 · 5 comments
Open

R2R image of SPC has many costly ValueTuple methods #76513

jakobbotsch opened this issue Oct 2, 2022 · 5 comments

Comments

@jakobbotsch
Copy link
Member

jakobbotsch commented Oct 2, 2022

I noticed that we spend a lot of JIT time and image size on ValueTuple methods in R2R:
https://gist.github.com/jakobbotsch/84a4525207268577fddb1b174b8ea63d

It seems these end up being rooted due to e.g.

/// <summary>
/// Make a properly nested <see cref="ValueTuple"/> from a properly nested <see cref="Tuple"/> with 21 elements.
/// </summary>
public static ValueTuple<T1, T2, T3, T4, T5, T6, T7, ValueTuple<T8, T9, T10, T11, T12, T13, T14, ValueTuple<T15, T16, T17, T18, T19, T20, T21>>>
ToValueTuple<T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19, T20, T21>(
this Tuple<T1, T2, T3, T4, T5, T6, T7, Tuple<T8, T9, T10, T11, T12, T13, T14, Tuple<T15, T16, T17, T18, T19, T20, T21>>> value)
{
return CreateLong(value.Item1, value.Item2, value.Item3, value.Item4, value.Item5, value.Item6, value.Item7,
CreateLong(value.Rest.Item1, value.Rest.Item2, value.Rest.Item3, value.Rest.Item4, value.Rest.Item5, value.Rest.Item6, value.Rest.Item7,
ValueTuple.Create(value.Rest.Rest.Item1, value.Rest.Rest.Item2, value.Rest.Rest.Item3, value.Rest.Rest.Item4, value.Rest.Rest.Item5, value.Rest.Rest.Item6, value.Rest.Rest.Item7)));
}

Skipping ToValueTuple and ToTuple methods avoids the root and reduces the size of SPC by around 2% on win-x64. It feels like a safe bet that most of these overloads are not going to be used in most applications, so skipping them would be fruitful in terms of deployment size.

It would also be nice to collect some data such as "X method is responsible for rooting Y bytes in the final image" and see if there are more cases like this that we should consider skipping.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 2, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@jakobbotsch jakobbotsch added this to the 8.0.0 milestone Oct 2, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 2, 2022
@jkoritzinsky
Copy link
Member

It would also be nice to collect some data such as "X method is responsible for rooting Y bytes in the final image" and see if there are more cases like this that we should consider skipping.

I think this idea will become even more interesting after #75793 gets in and we only root public (or possibly-public) surface area.

@jkotas
Copy link
Member

jkotas commented Oct 3, 2022

This leads to partially pre-compiling CoreLib using PGO data. I do not think we would want to maintain the list of what's not worth pre-compiling by hand.

We have experimented with partially compiling CoreLib based on PGO data in the past. Unfortunately, we have seen measurable regressions in number of cases. The problem is that each application uses a substantially different set of one-off methods from CoreLib.

We may want to redo the experiments with partially precompiling CoreLib to see where we would be landing with the current state of tiered JIT.

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Oct 3, 2022

I think certainly the heuristics of how generic methods are instantiated can be improved. For example, I don't think it makes sense to instantiate 21-ary value tuple method with 21 instances of System.__Canon without seeing any such caller.

@steveisok
Copy link
Member

I know this has had a few milestone bounces, but I think it would be interesting for us to look at in .NET 10.

@steveisok steveisok modified the milestones: 9.0.0, 10.0.0 Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants