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

Should JsonSerializerOptions have bounds in its cache size? #35084

Closed
jozkee opened this issue Apr 17, 2020 · 7 comments
Closed

Should JsonSerializerOptions have bounds in its cache size? #35084

jozkee opened this issue Apr 17, 2020 · 7 comments
Labels
area-System.Text.Json backlog-cleanup-candidate An inactive issue that has been marked for automated closure.
Milestone

Comments

@jozkee
Copy link
Member

jozkee commented Apr 17, 2020

What prevents the caches inside of this instance from growing unbounded? Do we need to place a limit on the size of the cache, making it an LRU cache or something like that?

Originally posted by @stephentoub in #35040 (comment)

cc @steveharter @layomia

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Apr 17, 2020
@ghost
Copy link

ghost commented Apr 17, 2020

Tagging subscribers to this area: @jozkee
Notify danmosemsft if you want to be subscribed.

@JamesNK
Copy link
Member

JamesNK commented Apr 17, 2020

What prevents the caches inside of this instance from growing unbounded?

Typically there are a finite number of types in an app that are serialized so it isn't an issue. But dynamic type generation plus serialization could create caching issues.

In Newtonsoft.Json the DefaultContractResolver holds the cache. Because it is an instance type it is possible to create a new contract resolver for serialization of dynamic types and then throw it away.

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Apr 17, 2020
@layomia layomia added this to the 5.0 milestone Apr 17, 2020
@NinoFloris
Copy link
Contributor

NinoFloris commented Apr 18, 2020

and then throw it away.

This is also important in the context of assembly unloading

@steveharter
Copy link
Member

As @JamesNK points out the existing caches are all rooted to a given Type that was previously (de)serialized.

Currently it is not possible to "unload" a given Type from the serialized, but that could certainly be done. For example, a JsonSerializerOptions.Uncache(Type) could be added. This would simply remove the JsonClassInfo entry for the Type from the dictionary held by the options class.

To unload all Types for a given assembly a JsonSerializerOptions.Uncache(Assembly) could be added which would enumerate the underlying dictionary to unload the appropriate types.

@NinoFloris
Copy link
Contributor

I understand the implications, I was pointing out assembly unloading as another angle to keep in mind.

@ghost
Copy link

ghost commented Oct 17, 2021

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of the experimental issue cleanup initiative we are currently trialing in a limited number of areas. Please share any feedback you might have in the linked issue.

@ghost
Copy link

ghost commented Nov 5, 2021

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.

@ghost ghost closed this as completed Nov 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 5, 2021
@eiriktsarpalis eiriktsarpalis added the backlog-cleanup-candidate An inactive issue that has been marked for automated closure. label Feb 18, 2022
@ghost ghost removed the no-recent-activity label Feb 18, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json backlog-cleanup-candidate An inactive issue that has been marked for automated closure.
Projects
None yet
Development

No branches or pull requests

7 participants