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

Possible linker removal of JsonDocument and JsonNode (+ ancillary classes) to save 35% of STJ.dll size #51756

Open
steveharter opened this issue Apr 23, 2021 · 3 comments
Labels
area-System.Text.Json size-reduction Issues impacting final app size primary for size sensitive workloads
Milestone

Comments

@steveharter
Copy link
Member

steveharter commented Apr 23, 2021

A prototype showed a 35% reduction of a trimmed System.Text.Json.dll with JsonDocument and JsonNode classes linked out. This also includes ancillary classes like JsonElement, JsonObject, JsonArray and JsonValue.

There are two scenarios for trimming: one with the JSON source generator used, and one without.

  • JsonValue is already trimmed (or should be) if the source generator is used, but not for the non-source generator case.
  • JsonDocument is never currently trimmed.

These can be safely trimmed out if:

  • [JsonExtensionData] is not used
  • Polymorphic deserialization to System.Object members is not used
  • The new JsonNode classes are not used
  • The JsonDocument and JsonElement types are not used.

To allow the non-source generator to safely link out JsonNode:

  • Hard reference on JsonExtensionDataAttribute to JsonElement and JsonNode to root these types when that feature is used.
  • Add a root dependency to JsonNode on the setter to JsonSerializerOptions.UnknownTypeHandling (default is to use JsonElement, so the setter must be called to change to JsonNode) or add alternative API that enables polymorphic (de)serialization in general (see below for potential breaking change).

To allow the source generator to link out JsonDocument the serializer dependencies can be addressed by:

  • A loosely typed converter model for these types, perhaps using reflection to instantiate the converter.
  • Hard references to these types replaced with string-based type name comparisons (similar to how this is done with JsonNode today).
  • The extension property code reworked to support late binding (similar to how this is done with JsonNode today).
  • The reference handling code reworked to support late binding

To allow the non-source generator cases we could introduce a breaking change for polymorphic deserialization of System.Object-declared properties\fields\elements:

  • Change the JsonSerializerOptions.UnknownTypeHandling to add a new default value of "none" (or similar new API).
  • Note that today a user can also specify JsonElement or JsonObject as a property\field\element instead of System.Object in order to be explicit and avoid having to set UnknownTypeHandling.
  • Note any API changes can be reconciled with the potential new support for true polymorphic deserialization being discussed for 6.0.
@steveharter steveharter added area-System.Text.Json linkable-framework Issues associated with delivering a linker friendly framework labels Apr 23, 2021
@ghost
Copy link

ghost commented Apr 23, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

A prototype showed a 35% reduction of a trimmed System.Text.Json.dll with JsonDocument and JsonNode classes linked out. This also includes ancillary classes like JsonElement, JsonObject, JsonArray and JsonValue.

These can be trimmed out if:

  • [JsonExtensionData] is not used
  • The new JsonNode classes are not used
  • The JsonDocument and JsonElement types are not used.

The reader\writer do not have any dependencies on these types.

The serializer does have dependencies on these but these can be addressed by:

  • A loosely typed converter model for these types, perhaps using reflection to instantiate the converter.
  • Hard references to these types replaced with string-based type name comparisons.
  • The extension property code reworked to support late binding
  • The reference handling code reworked to support late binding
  • Hard reference on JsonExtensionDataAttribute to JsonElement`JsonNode` to keep these types to support that feature.-
Author: steveharter
Assignees: -
Labels:

area-System.Text.Json, linkable-framework

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 23, 2021
@marek-safar marek-safar added size-reduction Issues impacting final app size primary for size sensitive workloads and removed linkable-framework Issues associated with delivering a linker friendly framework labels Apr 23, 2021
@ghost
Copy link

ghost commented Apr 23, 2021

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

Issue Details

A prototype showed a 35% reduction of a trimmed System.Text.Json.dll with JsonDocument and JsonNode classes linked out. This also includes ancillary classes like JsonElement, JsonObject, JsonArray and JsonValue.

These can be trimmed out if:

  • [JsonExtensionData] is not used
  • Polymorphic deserialization to System.Object members is not used
  • The new JsonNode classes are not used
  • The JsonDocument and JsonElement types are not used.

The reader\writer do not have any dependencies on these types.

The serializer does have dependencies on these but these can be addressed by:

  • A loosely typed converter model for these types, perhaps using reflection to instantiate the converter.
  • Opt-in to polymorphic deserialization of System.Object-declared properties\fields\elements.
    • Today a JsonElement is created to hold the values but that can be changed to JsonObject via the new JsonSerializerOptions.UnknownTypeHandling. We could add a new default value of "none".
    • Today a user can also specify JsonElement or JsonObject as a property\field\element instead of System.Object in order to be explicit and avoid having to set UnknownTypeHandling.
    • Also any API changes can be reconciled with the potential new support for true polymorphic deserialization being discussed for 6.0.
  • Hard references to these types replaced with string-based type name comparisons.
  • The extension property code reworked to support late binding
  • The reference handling code reworked to support late binding
  • Hard reference on JsonExtensionDataAttribute to JsonElement`JsonNode` to keep these types to support that feature.-
Author: steveharter
Assignees: -
Labels:

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

Milestone: -

@layomia layomia added this to the 6.0.0 milestone Apr 23, 2021
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Apr 23, 2021
@steveharter steveharter self-assigned this Aug 2, 2021
@steveharter
Copy link
Member Author

Since the main scenario for trimming is Blazor client, this suggestion will not help much since the JsonElement is used by Blazor. The bulk of JsonNode should already be trimmed for Blazor since it is not used.

Moving to 7.0 if we want to refactor Blazor to use Utf8JsonReader instead of JsonElement, so that JsonElement can be trimmed.

@steveharter steveharter modified the milestones: 6.0.0, 7.0.0 Aug 3, 2021
@steveharter steveharter removed their assignment Sep 23, 2021
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Development

No branches or pull requests

4 participants