NativeAOT SetCreateObject on JsonTypeInfo<T> could be slimmed down

[NinoFloris]

SetCreateObject on JsonTypeInfo<T> currently holds code to deal with the untyped/typed delegate and validation. This bloats every instantiation of JsonTypeInfo<T> while it should just do the bare minimum for the typed side to work.

It seems trivial to move the validation and untyped setter logic into a new non virtual method on JsonTypeInfo, this would then call into the abstract SetCreateObject to just do the last bits for the typed delegate in JsonTypeInfo<T>.

For specifically this path SetCreateObject would probably need to be changed to return its typed delegate as Func<object> (or Func (or null if it's not applicable) though it could also just overwrite _createObject.

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

Lines 86 to 90 in 46632f4

	else if (createObject is Func<T> typedDelegate)
	{
	typedCreateObject = typedDelegate;
	untypedCreateObject = createObject is Func<object> untypedDelegate ? untypedDelegate : () => typedDelegate()!;
	}

[ghost]

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

Issue Details

---

SetCreateObject on JsonTypeInfo<T> currently holds code to deal with the untyped/typed delegate and validation. This bloats every instantiation of JsonTypeInfo<T> while it should just do the bare minimum for the typed side to work.

It seems trivial to move the validation and untyped setter logic into a new non virtual method on JsonTypeInfo, this would then call into the abstract SetCreateObject to just do the last bits for the typed delegate in JsonTypeInfo<T>.

For specifically this path SetCreateObject would probably need to be changed to return its typed delegate (or null if it's not applicable) though it could also just overwrite _createObject.

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

Lines 86 to 90 in 46632f4

	else if (createObject is Func<T> typedDelegate)
	{
	typedCreateObject = typedDelegate;
	untypedCreateObject = createObject is Func<object> untypedDelegate ? untypedDelegate : () => typedDelegate()!;
	}

Author:	NinoFloris
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

It might help if you could share a PR that demonstrates the change. Thanks.

[ghost]

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

---

SetCreateObject on JsonTypeInfo<T> currently holds code to deal with the untyped/typed delegate and validation. This bloats every instantiation of JsonTypeInfo<T> while it should just do the bare minimum for the typed side to work.

It seems trivial to move the validation and untyped setter logic into a new non virtual method on JsonTypeInfo, this would then call into the abstract SetCreateObject to just do the last bits for the typed delegate in JsonTypeInfo<T>.

For specifically this path SetCreateObject would probably need to be changed to return its typed delegate as Func<object> (or Func (or null if it's not applicable) though it could also just overwrite _createObject.

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

Lines 86 to 90 in 46632f4

	else if (createObject is Func<T> typedDelegate)
	{
	typedCreateObject = typedDelegate;
	untypedCreateObject = createObject is Func<object> untypedDelegate ? untypedDelegate : () => typedDelegate()!;
	}

Author:	NinoFloris
Assignees:	-
Labels:	area-System.Text.Json, untriaged, linkable-framework
Milestone:	-

[krwq]

@NinoFloris please provide sample and potential saving (can be in terms of number of method i.e. or if you have numbers from mstat file - you can get mstat file using these instructions: #78671 (comment) and dump it using https://gist.github.com/eerhardt/9c56e6c60952adf8b463839e7670c7ce) then you can search for methods you think can be removed. Also make sure to show the sample app you run this on and show potential size saving in perspective of total.

I'm marking this as potential 8.0.

[ghost]

This issue has been marked needs-author-action and may be missing some important information.

[ghost]

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.