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

Potential size reduction opportunities in System.Text.Json NativeAOT #80917

Open
eiriktsarpalis opened this issue Jan 20, 2023 · 5 comments
Open
Assignees
Labels
area-System.Text.Json partner-impact This issue impacts a partner who needs to be kept updated size-reduction Issues impacting final app size primary for size sensitive workloads
Milestone

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 20, 2023

The PR in #80755 attempted to reduce the size of System.Text.Json components in trimmed NativeAOT binaries. The per-namespace breakdown in #80755 (comment) shows that there might be opportunity to further reduce application size in the System.Text.Json.Serialization.Metadata namespace, which currently occupies approximately 600k in the Golidlocks app. More specifically, the metadata layer introduces overhead in a couple of ways:

  1. Using separate JsonTypeInfo<T> implementations for source gen and reflection. This segregation largely exists for historical reasons and it could be removed, decoupling JsonTypeInfo<T> metadata from contract resolution behavior.
  2. The source generator sets JsonTypeInfo metadata using the JsonMetadataServices APIs as a proxy. This happens because the source generator predates the public APIs on JsonTypeInfo itself. The proxy API introduces a number of generic classes such as JsonCollectionInfoValues<T>, JsonObjectInfoValues<T> and JsonPropertyInfoValues<T>. All these types could be trimmed if we updated the source generator to call into the JsonTypeInfo APIs directly. Note that this is currently blocked by Add support for parameterized constructors in System.Text.Json contract customization (converters) #71944, the only remaining configuration point available in JsonMetadataServices but not yet present in JsonTypeInfo<T>.

cc @eerhardt @krwq @layomia

@eiriktsarpalis eiriktsarpalis self-assigned this Jan 20, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 20, 2023
@eiriktsarpalis eiriktsarpalis removed their assignment Jan 20, 2023
@ghost
Copy link

ghost commented Jan 20, 2023

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

The PR in #80755 attempted to reduce the size of System.Text.Json components in trimmed NativeAOT binaries. The per-namespace breakdown in #80755 (comment) shows that there might be opportunity to further reduce application size in the System.Text.Json.Serialization.Metadata namespace. More specifically, the metadata layer introduces overhead in a couple of ways:

  1. Using separate JsonTypeInfo<T> implementations for source gen and reflection. This segregation largely exists for historical reasons and it could be removed, decoupling JsonTypeInfo<T> metadata from contract resolution behavior.
  2. The source generator sets JsonTypeInfo metadata using the JsonMetadataServices APIs as a proxy. This happens because the source generator predates the public APIs on JsonTypeInfo itself. The proxy API introduces a number of generic classes such as JsonCollectionInfoValues<T>, JsonObjectInfoValues<T> and JsonPropertyInfoValues<T>. All these types could be trimmed if we updated the source generator to call into the JsonTypeInfo APIs directly. Note that this is currently blocked by Add support for parameterized constructors in System.Text.Json contract customization (converters) #71944, the only remaining configuration point available in JsonMetadataServices but not yet present in JsonTypeInfo<T>.
Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis eiriktsarpalis added this to the Future milestone Jan 20, 2023
@eiriktsarpalis eiriktsarpalis added the size-reduction Issues impacting final app size primary for size sensitive workloads label Jan 20, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 20, 2023
@ghost
Copy link

ghost commented Jan 20, 2023

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

The PR in #80755 attempted to reduce the size of System.Text.Json components in trimmed NativeAOT binaries. The per-namespace breakdown in #80755 (comment) shows that there might be opportunity to further reduce application size in the System.Text.Json.Serialization.Metadata namespace. More specifically, the metadata layer introduces overhead in a couple of ways:

  1. Using separate JsonTypeInfo<T> implementations for source gen and reflection. This segregation largely exists for historical reasons and it could be removed, decoupling JsonTypeInfo<T> metadata from contract resolution behavior.
  2. The source generator sets JsonTypeInfo metadata using the JsonMetadataServices APIs as a proxy. This happens because the source generator predates the public APIs on JsonTypeInfo itself. The proxy API introduces a number of generic classes such as JsonCollectionInfoValues<T>, JsonObjectInfoValues<T> and JsonPropertyInfoValues<T>. All these types could be trimmed if we updated the source generator to call into the JsonTypeInfo APIs directly. Note that this is currently blocked by Add support for parameterized constructors in System.Text.Json contract customization (converters) #71944, the only remaining configuration point available in JsonMetadataServices but not yet present in JsonTypeInfo<T>.
Author: eiriktsarpalis
Assignees: -
Labels:

area-System.Text.Json, untriaged, size-reduction

Milestone: Future

@eiriktsarpalis eiriktsarpalis added the partner-impact This issue impacts a partner who needs to be kept updated label Jan 20, 2023
@krwq
Copy link
Member

krwq commented Jan 20, 2023

Shouldn't the split into source-gen/reflection help eventually? I'd assume if you use source-gen all ReflectionJsonTypeInfo should get trimmed away (and reverse). Should we strive into that actually happening if it's not currently?

@krwq
Copy link
Member

krwq commented Jan 20, 2023

Other option we can move all the reflection logic to DefaultJsonTypeInfoResolver and keep JsonTypeInfo clean. That should achieve same effect and might actually be easier to achieve

@eiriktsarpalis
Copy link
Member Author

Shouldn't the split into source-gen/reflection help eventually? I'd assume if you use source-gen all ReflectionJsonTypeInfo should get trimmed away (and reverse). Should we strive into that actually happening if it's not currently?

The derived types aren't necessary and because they are generic they contribute to a linear increase of application size in NativeAOT apps. There should eventually be one sealed JsonTypeInfo<T> class that each resolver populates independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json partner-impact This issue impacts a partner who needs to be kept updated size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Development

No branches or pull requests

2 participants