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

Reuse JsonSerializerOptions when used with JsonSerializerContext #64025

Closed
JuanZamudioGBM opened this issue Jan 20, 2022 · 9 comments
Closed
Assignees
Labels
area-System.Text.Json needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration source-generator Indicates an issue with a source generator feature
Milestone

Comments

@JuanZamudioGBM
Copy link

Description

We currently have an extension method to encapsulate the serialization/deserialization logic, this is just basically because we have a custom converter that serialize enums as strings, we have a private static field of JsonSerializerOptions that we reuse when we call JsonSerializer.Serialize to avoid generating a new object in every call.

We are testing the source generator in System.Text.Json and tried to do the same, but we got the exception:
System.InvalidOperationException: "The specified 'JsonSerializerOptions' instance is already bound with a 'JsonSerializerContext' instance."

The call is basically this:
JsonSerializer.Serialize(myObject, typeof(MyObjectDto), new MyContext(StaticJsonSerializerOptions));

Is there a way to avoid creating a new instance of JsonSerializerOptions in every call? or is it something wrong on how we are approaching this?

Configuration

.NET SDK 6.0.101

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Jan 20, 2022
@ghost
Copy link

ghost commented Jan 20, 2022

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

Issue Details

Description

We currently have an extension method to encapsulate the serialization/deserialization logic, this is just basically because we have a custom converter that serialize enums as strings, we have a private static field of JsonSerializerOptions that we reuse when we call JsonSerializer.Serialize to avoid generating a new object in every call.

We are testing the source generator in System.Text.Json and tried to do the same, but we got the exception:
System.InvalidOperationException: "The specified 'JsonSerializerOptions' instance is already bound with a 'JsonSerializerContext' instance."

The call is basically this:
JsonSerializer.Serialize(myObject, typeof(MyObjectDto), new MyContext(StaticJsonSerializerOptions));

Is there a way to avoid creating a new instance of JsonSerializerOptions in every call? or is it something wrong on how we are approaching this?

Configuration

.NET SDK 6.0.101

Author: JuanZamudioGBM
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@layomia
Copy link
Contributor

layomia commented Jan 20, 2022

JsonSerializer.Serialize(myObject, typeof(MyObjectDto), new MyContext(StaticJsonSerializerOptions));

Can you cache & reuse the context instance that you create, similar to how you're already caching your custom options?

StaticJsonSerializerContext = new MyContext(StaticJsonSerializerOptions);
...
JsonSerializer.Serialize(myObject, typeof(MyObjectDto), StaticJsonSerializerContext);

@layomia layomia added needs-author-action An issue or pull request that requires more info or actions from the author. and removed untriaged New issue has not been triaged by the area owner labels Jan 20, 2022
@ghost
Copy link

ghost commented Jan 20, 2022

This issue has been marked needs-author-action since it may be missing important information. Please refer to our contribution guidelines for tips on how to report issues effectively.

@JuanZamudioGBM
Copy link
Author

JuanZamudioGBM commented Jan 20, 2022

It does work, but it needs a new instance of JsonSerializerOptions per JsonSerializerContext (I just found out that i can reuse the context with multiple types and have just one JsonSerializerContext), and our benchmarks shows that the performance is almost identical to the reflection based serialization (in speed and memory allocation) so we will use the current solution at the moment.

Thanks a lot for your time @layomia

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 20, 2022
@layomia
Copy link
Contributor

layomia commented Jan 21, 2022

@JuanZamudioGBM thanks for your feedback. Could you share a repro of your benchmarks so we can see why there are no perf improvements for serialization in your scenario? Note that there are no expected throughput improvements for deserialization with the current source-gen implementation.

@layomia layomia added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Jan 21, 2022
@ghost
Copy link

ghost commented Jan 21, 2022

This issue has been marked needs-author-action since it may be missing important information. Please refer to our contribution guidelines for tips on how to report issues effectively.

@layomia layomia added the source-generator Indicates an issue with a source generator feature label Jan 21, 2022
@JuanZamudioGBM
Copy link
Author

Let me prepare a repro and upload it to a public account.

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 24, 2022
@JuanZamudioGBM
Copy link
Author

Here is the repro, I hope its useful.

https://github.com/JuanZamudioGBM/source-generator

@layomia layomia self-assigned this Mar 7, 2022
@layomia layomia added this to the 7.0.0 milestone Mar 7, 2022
@eiriktsarpalis
Copy link
Member

With the inclusion of #63686 in .NET 7 it should be possible to assign existing JsonSerializerContext instances to new JsonSerializerOptions instances via the TypeInfoResolver property. It will also be possible to combine multiple JsonSerializerContext sources into one contract resolver as follows:

IJsonTypeInfoResolver combinedResolver = JsonTypeInfoResolver.Combine(MyContext1.Default, MyContext2.Default, MyContext3.Default);
var options = new JsonSerializerOptions { TypeInfoResolver = combinedResolver };

The new options instance will be combining source generated metadata from all three sources.

Per @layomia's earlier comment, using the metadata-based source generator is not expected to produce any substantial performance gains compared to reflection. The purpose of the metadata-based source generator is to minimize static initialization time and support application trimming.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

3 participants