Runtime: Api proposal: Change JsonSerializerOptions default settings

[andre-ss6]

New API:

public static class JsonSerializerOptions
{
    // Can only be set until the first (de)serialization occurs (like other properties here)
    public static Func<JsonSerializerOptions> Default { get; set; }
}

Newtonsoft \ JSON.NET does it this way:

public static class JsonConvert
{
    public static Func<JsonSerializerSettings> DefaultSettings { get; set;}
}

The upside is that you can have different settings for different situations, including different options per assembly, even though that would require the developer be aware of the potential pitfalls of that. The fact that it has also worked for many years in JSON.NET I think is a plus as well.

Performance could suffer a little bit, would have to test. Either way, one could always get over that by explicitly passing their options.

---

Original text:

Provide a way to change the default settings for the static JsonSerializer class. In Json.NET, you could do:

JsonConvert.DefaultSettings = () => new JsonSerializerSettings();

Currently you have to keep passing the options around and remembering to pass it to each call to [De]Serialize

[ahsonkhan]

Currently you have to keep passing the options around and remembering to pass it to each call to [De]Serialize

Can you expand on why that is a concern for you and what scenario is blocked by the current design (preferably with some sample code)?

Given the JsonSerializer is a static class containing purely static methods, it doesn't contain any state atm. Providing rationale for why this needs to change would help motivate it (other than the fact that Json.NET has this behavior) since it does introduce some complexity in the design.

[andre-ss6]

Can you expand on why that is a concern for you

JSON properties are conventionally camelCase. .NET type's members, on the other hand, are conventionally PascalCase. I don't really want to stray away from any of these two conventions.

The solutions today, as far as I'm aware, all involve passing things around:

1) You can annotate every single property in your project with JsonPropertyNameAttribute. This is not nice, convenient or even desirable. [JsonPropertyName("foo")] int Foo { get; set; } adds unnecessary verbosity and feels and looks completely redundant. The only thing changing between the two lines is the first character casing. It is also error prone.
2) Pass options around everywhere. I think this one speaks for itself. If you're passing options around _everywhere_, you should consider instead doing it in one place only. This is the path we're following with our project, which I talk more about in the end.
3) Pass a serializer around everywhere as well. Although this one seems to make some sense at first (oh, you need JSON serialization? Ask the IoC for a serializer!), it does make you wonder: why have a static helper class in the first place then? As far as I'm aware, this requires writing code considerably more involved than simply calling JsonSerializer.Desearilize<T>("..."). So you won't be writing conventional code just because you need different options. And again, error prone.

Now, of course one could argue that the correct way is indeed to have case-sensitivity, since you can always have JSON like { "foo": 1, "Foo": 2}. However, being honest, this is not a common case -- and not only that, but I don't really see people writing this kind of JSON and expecting it to work 100% in the wild. Of course it's on the spec, but realistically it will simply break lots of APIs out there. Finally, who governs the names of accepted properties is the API, thus, our API obviously do not do such things. In fact, creating classes like that would go against the .NET design guidelines.

A better option, perhaps, would be to inform the serializer that by default all properties are to be assumed camelCase (so, they would remain case-sensitive, but by default be assumed JSON camelCase even though being written PascalCase in C#). However, the problem here is the "by default" bit, which again boils down to the same problem exposed in this issue.

_All_ of our projects currently expect this behavior. Changing this wouldn't be a deal breaker for adopting System.Text.Json, but it is surely requiring some unexpected changes. The first change we did was to make our REST internal library now accept a JsonSerializerOptions in its constructor so that it can be set during DI registration. However, there are a considerable number of other places in the project where we use JSON serialization for other purposes. For those other scenarios, the only idea we currently have is to pass options around.

If there was an instance, non-static version of JsonSerializer we could also make extension methods. This unfortunately still wouldn't change the behavior of the entire running program, but would at least change the behavior for our assembly.

[Symbai]

Why don't you declare the default setting in a static class and then just do

static class DefaultJsonSettings {
    public static readonly JsonSerializerSettings Settings = new JsonSerializerSettings {
        NullValueHandling = NullValueHandling.Ignore,
        // [...]
    };
}

 JsonConvert.SerializeObject(myObject, DefaultJsonSettings.Settings);

That's what I did to avoid passing settings around but making sure it's getting serialized the same everywhere in project. If your wish is to set the settings ONCE at start you already know how the settings should be so static is fine. Otherwise if you load the settings, so they are unknown before application start, it cannot be static itself but you could do the same but with a static reference to an instance:

public sealed class DefaultJsonSettings  {
    static DefaultJsonSettings instance;

    public static DefaultJsonSettings Instance => instance ??= new DefaultJsonSettings();

    public DefaultJsonSettings() {
        //Load JSON settings here
        Settings = new JsonSerializerSettings {
            NullValueHandling = NullValueHandling.Ignore,
            // [...]
        };
    }

    public static JsonSerializerSettings Settings;
}
 JsonConvert.SerializeObject(myObject, DefaultJsonSettings.Instance.Settings);

[andre-ss6]

Sure, but that still kind of have the same issues as passing it around everywhere, except they're not a method parameter.

You still have to remember to do it, and thus it's still error prone, and it still feels quite redundant to be passing the "default" options to every single method call. If it's supposed to be the default, why do I need to specify it every single time? It's like having C# optional parameters, but still having to specify them in every call.

[Symbai]

why do I need to specify it every single time?

Because of what ahsonkhan said. Its a static class with static methods for performance reasons. You want add performance decreases by making the method not static only to avoid remembering to add "DefaultJsonSettings.Settings" every time you use it. This seems like a really bad trade, especially for the great majority where the current way it works isn't a big deal.

If this is such a big deal for you, why don't you just write DefaultJsonSettings.Settings once and put that inside a static method and call this instead, problem solved.

static class Bla {
    public string SerializeObject(object myObject)
    {
        return JsonConvert.SerializeObject(myObject, DefaultJsonSettings.Settings);
    }
}

Bla.SerializeObject(myObject);

And please do not say you don't want this because you have to remember using Bla.SerializeObject instead of JsonConvert.SerializeObject

[andre-ss6]

And please do not say you don't want this because you have to remember using Bla.SerializeObject instead of JsonConvert.SerializeObject

How so "please do not say"? Am I not allowed to express my opinion? By saying so, you show that you do understand the issues I'm raising and you do understand that they're present in the ""solution"" you suggest. That's dishonest. Remember that different people have different needs, works in different projects and under different scenarios. Your opinion is not golden. It's just as valid as mine. If my suggestion doesn't prove to be much useful, don't worry, it simply won't get traction and won't get implemented. Don't think there's a need for you to harass anyone.

You want add performance decreases

I want no such thing. Never said it. I'm merely asking for a way to have default settings. The implementation is up for discussion.

only to avoid remembering to add "DefaultJsonSettings.Settings" every time you use it

It's not "only" to avoid remembering. When you argue like that, you can make any argument look pointless. As I said in previous replies, this is simply a point of great coding practices. Anyone who owns a codebase and see a thing such as having a method being called all around the solution every time with the same argument would do something about it. And again, this is because it is simply a point of great coding practices. It's easy to see the point.

This seems like a really bad trade

Only if it has to be that way. I too agree that if a non-insignificant performance impact is _required_ in order to do this, it wouldn't pay itself, and I wouldn't support it.

---

@ahsonkhan Can't we just create a new overload that doesn't have the JsonSerializerSettings optional parameter and that just redirects the call, passing the DefaultSettings as parameter? Sure, that would introduce state in the class, but I don't understand why is that a problem _per se_. And AFAIK that wouldn't be a breaking change either if the DefaultSettings property is default initialized with null. I'm sorry if there's other complexity involved that I'm not aware, I haven't looked at the code yet for that class.

[andre-ss6]

Just as a quick note, I think it's also worth remembering that another benefit of having a static property is that you're able to change the serializer behavior for all assemblies in the running program. I don't depend on this behavior currently though, so I can't say how much of a benefit that would be. I guess it _could_ be useful in scenarios where you have code over which you have no control that uses JsonSerializer internally and doesn't offer an way to change the default options.

[FiniteReality]

I really think this is a huge trade-off. It can be a huge pit-of-failure in libraries which interact with web APIs which expect a specific casing. Of course, they should be written in a way which is not vulnerable to this, but by making it the "default", it makes it a lot easier for bad code to be written.

Personally, I don't really think specifying JsonPropertyName for every property name is a negative - in fact, I feel that explicitly specifying property names is advantageous, even if it can seem like code duplication.

[andre-ss6]

Of course, they should be written in a way which is not vulnerable to this, but by making it the "default", it makes it a lot easier for bad code to be written.

I understand your point. However, I'm not sure if you misunderstood me or if I'm misunderstanding you 😄 but I'm not advocating for that behavior to be _the_ default; instead, only to give users the option to have _a_ default, whatever that might be.

Having said that, please note then that 1) you're focusing on case-sensitivity, when that was merely the reasoning behind _my_ need for this. This issue is not about case sensitivity, it is about offering a way to change the defaults only once. No one is asking here for case-insensitive to be the default behavior anywhere and of anything; and 2) your point boils down to "giving developers a choice means they will write bad code and thus let's not give them a choice". And although I can see an argument like that being applied elsewhere, I don't really think it applies here.

I'm still going to reply to each one of your points, but I think this (whether or not case-sensitivity should be the default) is mostly something _not_ worth arguing over, as it is not the point of _this_ issue.

---

Please note that this has existed in JSON.NET for many many years and I don't think a lot of "bad" code has been written because of it. In fact, in JSON.NET, that behavior (case-insensitive) is _the_ default behavior (but again, I am not saying it should be, I'm merely bringing that fact to light).

It can be a huge pit-of-failure in libraries which interact with web APIs which expect a specific casing

First of all, notice that I'm more interested in the _deserialization_ behavior than the _serialization_ one. I don't really care how the payload is going to be serialized.

As said above, 1) I am not advocating for this to be _the_ default behavior; and 2) this has existed for years and I don't really see this ever being a problem. And, in fact, as I said before, I actually think it's the other way around: .NET developers are more familiar with cC -> CC because of the clash between the .NET naming guidelines and JS naming conventions. And that's not really a problem at all, since, as mentioned before, people simply don't go around there sending "foo: 1, "Foo": 2 JSON payloads.

Also, please note that this is the default behavior for MVC (and still continues to be, even after System.Text.Json: https://github.com/aspnet/AspNetCore/issues/10723) and, again, people are not writing "bad" code because of it. And, most importantly, MVC was engineered in a way where you can, in fact, change the default only once.

I really think this is a huge trade-off.

So I don't think there is really any trade-off here (in the context of what you said -- note that I do understand that there may be some trade-offs in terms of performance and/or API design!).

By having the option to specify the defaults in System.Text.Json, not only it would give us more flexibility, since - and this is important to note - there are other behaviors besides case-sensitivity that can be controlled through the options, but it would also pave the way for a more seamless migration between the two libraries.

Personally, I don't really think specifying JsonPropertyName for every property name is a negative - in fact, I feel that explicitly specifying property names is advantageous, even if it can seem like code duplication.

Right. I'm a big advocate of the idea of being explicit. However:

1) I do not like the idea of specifying _behavior_ in that way. One thing is having a property with a name that is not a valid C# identifier and having to enable that scenario through the use of an attribute. Another thing entirely is peppering your entire code with JsonPropertyNameAttributes everywhere, specifying through the use of _a string_ that the behavior you expect is "transformation" of camelCase to PascalCase. The behavior is encoded not in code, but in the developer's mind. [JsonPropertyName("foo")] string Foo { get; set; } doesn't really say what you're meaning. A future developer entering your project might not notice at first hand that that is a behavior expected by the entire project, instead of being just a few attributes here and there. On the other hand, something like Options.CaseInsensitive = true is explicit _and_ conveys all the meaning needed. By only looking at a single line such as that, a future developer can understand how your project deals with JSON.
2) It is brittle. You can't use nameof. You can't specify behavior, as said above (like something as [JsonProperty(JsonSerializationOptions.CaseInsensitive)] would). And logic is smeared all over your code, instead of of one place. Which leads me again to the obvious next point, also addressing one of yours;
3) It _is_ duplication. It not only looks like it. It is behavior what you're describing when writing that attribute and you're specifying it literally _everywhere_. Again, even though I _do_ love the idea of being explicit, I just can't feel that the advantages there cover the disadvantages of having to use attributes everywhere, _especially_ in a situation like this, with all these other mentioned disadvantages. We've seen this before many many times in the BCL and other MS frameworks (cough DataMemberAttribute cough). It is not by chance that they strayed away from this philosophy.

[steveharter]

The feature ask is valid IMO and was discussed early on when designing JsonSerializerOptions.

Currently the "default" option is on a private static variable. This means it can only contain default settings.

Also, FWIW, we also discussed add assembly-level attributes that can configure things like the naming policy. This would support some of the scenarios without having to specify where the default option lives.

[andre-ss6]

((JsonSerializerOptions)typeof(JsonSerializerOptions)
                .GetField("s_defaultOptions", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic).GetValue(null))
            .PropertyNameCaseInsensitive = true;

😈😈😈

[Alexei-B]

The solution @andre-ss6 came up with above using the internal default settings demonstrates that this is a real issue.
Can we get some movement on this? It's a pain to have to deal with libraries using System.Text.Json that don't let me configure these options.

[AndrewZenith]

Absolutely insane that you can't just specify PascalCase like in Newtonsoft's. Guess we'll just stick to Newtonsoft's unless common sense prevails.

Did you know Visual Studio by default moans at me if I use camelcase for properties in C#. Joined up thinking this isn't.

[francescocristallo]

I have the same issue, I'm converting a Core 2.2 project to 3.0 and the APIs are called from a Javascript file that requires Pascal case for the JSON returned; would be great to have a global option in Startup.cs to specify Pascal case for all the Serializations.

[ahsonkhan]

@AndrewZenith, @francescocristallo - the request to allow setting "PascalCase" naming policy in the options is separate from the discussion here in this issue, which is discussing adding the capability to enable setting the json serializer options globally, once.

Can you please file a separate issue for this (in the dotnet/runtime repo - https://github.com/dotnet/corefx/issues/42733), including details on your specific requirement and why the current behavior doesn't work.

The default behavior of System.Text.Json is that the .NET POCO properties are serialized using the exact property name. Similarly, deserialization requires an exact string match within the JSON payload (casing and format). In .NET/C#, since your object graph generally contains pascal cased properties, and the JSON being serialized is pascal-cased as well. Alternatively, you could apply the custom JsonPropertyName attribute to them.
https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializeroptions.propertynamingpolicy?view=netcore-3.0

We added camel case supported in 3.0 and are considering adding snake case support for 5.0: https://github.com/dotnet/corefx/issues/39564

I am going to mark this conversation as off-topic.

[jdaaboul]

It should be a given that .net let us set default settings for the serializer, having to repeat the options everywhere or use wrapper classes is too error prone and will cost more than slight performance regressions. Newtonsoft it is until this is fixed.

[AndrewZenith]

@AndrewZenith, @francescocristallo - the request to allow setting "PascalCase" naming policy in the options is separate from the discussion here in this issue, which is discussing adding the capability to enable setting the json serializer options globally, once.

Can you please file a separate issue for this (in the dotnet/runtime repo - dotnet/corefx#42733), including details on your specific requirement and why the current behavior doesn't work.

The default behavior of System.Text.Json is that the .NET POCO properties are serialized using the exact property name. Similarly, deserialization requires an exact string match within the JSON payload (casing and format). In .NET/C#, since your object graph generally contains pascal cased properties, and the JSON being serialized is pascal-cased as well. Alternatively, you could apply the custom JsonPropertyName attribute to them.
https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializeroptions.propertynamingpolicy?view=netcore-3.0

We added camel case supported in 3.0 and are considering adding snake case support for 5.0: dotnet/runtime#782

I am going to mark this conversation as off-topic.

But setting PascalCase ONCE in the STATIC json serializer options is what I and everyone else moving from MVC5 to Core 2.2 then onto 3.0 will want to do. Because we've already written the code like that (PascalCase in C#, camelCase in Javascript), have an awfully large number of classes it affects and have no intention of going round and applying another attribute to everything and hoping we got them all. It's EXACTLY what the op is asking for, so very on topic.

I also just want to point out to those coming here that there is an option, which is NewtonSoft JSON can be added back in as middleware and be set to work exactly as it used to, so we can move to .Net Core 3.0 regardless of this substantial oversight by kicking out this implementation of a Json Serializer. Don't get me wrong, I'd like to use it, but without what the op has asked for - very unlikely.

[dquist]

Ran into this issue when trying to create an asp.net web API with matching .net core console client.

The default behavior of the serializer is to use camelCase, but the default deserialization option is PascalCase. Had to google around as to why .net core couldn't deserialize an object I just serialized using the same model 🤔.

Passing in the same options value across the entire application seems like a huge violation of DRY and an easy way for bugs to creep in.

[ahsonkhan]

Ran into this issue when trying to create an asp.net web API with matching .net core console client.

The default behavior of the serializer is to use camelCase, but the default deserialization option is PascalCase. Had to google around as to why .net core couldn't deserialize an object I just serialized using the same model 🤔.

The default behavior of System.Text.Json.JsonSerializer is to match the same name/casing as the .NET property name (the names in the .NET POCO) for both serialize and deserialize:

https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializeroptions.propertynamingpolicy?view=netcore-3.0

Within aspnet however (which includes Mvc, SignalR, Web Api), the JsonSerializer is configured with camel casing by default, along with case insensitive matching: https://github.com/aspnet/AspNetCore/blob/a6bc6ce23dad5a9d02596cf2e91e3c4f965c61bc/src/Mvc/Mvc.Core/src/JsonOptions.cs#L28

https://github.com/aspnet/AspNetCore/blob/ec8304ae85d5a94cf3cd5efc5f89b986bc4eafd2/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs#L747-L763

That is likely why you are seeing the observing the discrepancy. Aspnet configures the defaults differently.

If your .NET object graph happens to contain PascalCase properties (which it most likely does so), then yes, you would need to configure your JsonSeriaizerOptions to be consistent across your asp.net web API and the .net core console client, because the defaults in both contexts are different.

You would have to override the options in either in your web api or in your console client to make sure they are consistent (you probably want your console clients json serializer options to be configured to match the web api serializer options). Would having a globally configurable serializer help in your scenario? If the two contexts are in separate assemblies, you would have to configure both anyway, no?

[jtreher]

I just wanted to add a comment here that adding case-insensitivity appears to increase memory consumption and degrade performance on very large payloads. Since there does not appear to be a way to turn off case sensitivity with Newtonsoft deserialization, I am unable to compare to that with it disabled.

My memory as reported by BenchmarkDotNet was 2.5MB deserializing this massive stream and case insensitive enabled it went to 5.5MB. Performance was 20ms without and 26ms with.

For comparison, Newtonsoft is currently taking 32ms to deserialize the stream and consumes 3.5mb.

Default buffer size is set to 100KB.

[andre-ss6]

@jtreher can you share the code for that benchmark?

[jtreher]

@andre-ss6 My payload has a lot of data in it that would need scrubbed. The benchmark is pretty simple. You can either set the case insensitive in the benchmark method or just flip the flag and run the benchmarks again and you'll see the decline in performance insensitive vs. sensitive. My "response.json" is 1.5 MB for reference -- 100 very large and several nested levels deep objects.

    [MemoryDiagnoser]
    public class Test
    {
        private static readonly utf8json.JsonSerializerOptions _jsonOptions = new utf8json.JsonSerializerOptions
        {
            IgnoreNullValues = true,
            PropertyNameCaseInsensitive = true,
            DefaultBufferSize = 100_000,
        };

        private static string Contents;

        public Test()
        {
            using StreamReader sr = new StreamReader("C:/temp/response.json");
            Contents = sr.ReadToEnd();
        }

        [Benchmark]
        public async ValueTask<IReadOnlyCollection<ProductResponse>> SystemTextJsonDeserializeStream()
        {
            using StreamReader sr = new StreamReader("C:/temp/response.json");
            return await utf8json.JsonSerializer.DeserializeAsync<IReadOnlyCollection<ProductResponse>>(sr.BaseStream, _jsonOptions);
        }

        [Benchmark]
        public IReadOnlyCollection<ProductResponse> SystemTextJsonDeserializeObject()
        {
            return utf8json.JsonSerializer.Deserialize<IReadOnlyCollection<ProductResponse>>(Contents, _jsonOptions);
        }
}

[tdreid]

Sincere question — how much design complexity would be introduced from this one particular proposed divergence from a static class containing purely static methods? I acknowledge there would be trade offs. I'd like to understand their magnitude.

On a separate note, a few different ways to approach setting the defaults in one place have been suggested on the related Stack Overflow question.

[andre-ss6]

As I understand it, the tension is more on the design side than performance considerations. I think they're not sure yet if they want to expose a static property.

[tdreid]

Thanks @andre-ss6. I had misread @ahsonkhan's comment at the head of the thread. I've updated my inquiry to reflect the more pertinent trade off.

[andre-ss6]

Yea, it would be nice if they share that info with us. Simply saying there is complexity involved but then leaving us in the dark is not helpful.

I would hope we could help in the discussion. Simply requesting a feature and waiting in the dark feels like UserVoice.

[andre-ss6]

Alright, taking a stab at it.

Static defaults property (publicly expose today's field):

• Will change the default behavior for every single loaded assembly _unconditionally_ 👎;
  
  
  
  • However, library authors who need specific behaviors should know that they are not to trust in a public static property, and thus pass their own defaults every time to every method 💭.
  
  

Static generator property (JSON.NET):

• _Can_ change the defaults for every single loaded assembly 👍;
  
  
  
  • Although this would be _technically_ supported, it still may suffer from the same issues as the first option above 👎.
  
  
• Will incur an indirect method call for every [De]serialize call that don't specify defaults, thus affecting performance 👎.
  
  
  
  • We could cache the value as long as the property setter is never called, but complexity quickly arises when you start thinking about thread safety. Maybe JNK has already solved this in his library? 💭
  
  

Assembly attribute:

• The worst option? Unclear and low discoverability (if that's a word); 👎
• May or may not change behavior for all loaded assemblies depending ok implementation;
• May or may not badly affect performance depending on implementation;
• Needs design;

New C# 8.1 feature where you tell the compiler that you want a optional parameter to always have the same value:

• Would probably get more dev team attention than this issue. 👍

[ahsonkhan]

Yea, it would be nice if they share that info with us. Simply saying there is complexity involved but then leaving us in the dark is not helpful.

I would hope we could help in the discussion. Simply requesting a feature and waiting in the dark feels like UserVoice.

Apologies for that. I had been working through other JSON issues, since the backlog of feature requests is relatively large (and as you probably know, context switching is difficult). I will try to be more responsive here.

Can you update the OP with the exact API shape/change you'd like to propose that will address your concern and then we can discuss it. Once ready-for-review, we can take it to API review in the new year.
https://github.com/dotnet/runtime/blob/master/docs/project/api-review-process.md
Here's a good example of an API proposal: https://github.com/dotnet/corefx/issues/271

Having the caller override the default JsonSerializerOptions assembly-wide (or I guess, it would be application wide - like you said for every single loaded assembly _unconditionally_), may be relatively easy to achieve on paper, but it would have ramifications across the entire stack (at the very least, it raises questions that need to be thought through). That's partially what I meant with introduction of complexity. Now, the previous invariants, that depend on the default options not changing, would have to be reconsidered.

I have a few questions that are worth looking into as part of the spec/requirements:
1) What is the behavior when someone creates a "global" default but then wants to override that?
- Override through locally defined options
- Override through attributes/converters/etc.
2) Currently, the JsonSerializerOptions are immutable once serialization has started. Would this change affect that?
3) Would this affect converters in any way? I am assuming it wouldn't.

Can't we just create a new overload that doesn't have the JsonSerializerSettings optional parameter and that just redirects the call, passing the DefaultSettings as parameter?

4) Overloads on what? The existing Serialize/Deserialize methods? What would these API additions look like and could they cause overload resolution to break in some cases?

There is clearly a vocal interest here. Would you (@andre-ss6 ) like to take a shot at an implementation as part of the design, especially since you have already put a lot of thought into this? While we wait for the API to get reviewed, I would be happy to take a look at a draft/WIP PR which helps showcases the feature in action along with tests to help design this. I would only do this after we have consensus on the this thread around the api proposal and it is marked as api-ready-for-review.

Moving this from future to 5.0.

[ahsonkhan]

I just wanted to add a comment here that adding case-insensitivity appears to increase memory consumption and degrade performance on very large payloads.

@jtreher, were you running on 3.0 or 3.1? Can you try running them again on 5.0 and share your results, especially the difference in memory allocation? Also, is the sample model/payload representative of what you actually use and see in practice?

I understand you can't share the payload, but can you share your ProductResponse model (possibly sanitized if needed)? The shape/types would help narrow down why you are seeing such a difference. Does the delta only occur during deserialization (or does it happen during serialization as well)? A dummy/"auto-generated" payload that fits the schema would also help if you can create one, but as long as I have the model, I can do that myself.

My memory as reported by BenchmarkDotNet was 2.5MB deserializing this massive stream and case insensitive enabled it went to 5.5MB. Performance was 20ms without and 26ms with.

I am asking because I tried deserializing a 150 MB payload from NuGet into a SearchResult model (which isn't too deep), and though I do see a similar throughput difference as you when opting into `PropertyNameCaseInsensitive, I couldn't see the 2x+ difference in allocations like yours (2.5 MB to 5.5 MB). I saw a ~25% increase.
Something like the following:
https://github.com/NuGet/NuGet.Services.Metadata/blob/1a9a77014f4c477b222844c60ff549b93b6d5b4c/src/NuGet.Services.AzureSearch/SearchService/Models/V3SearchPackage.cs#L12

The perf difference is expected given how the caching of property names works currently. We have certain optimizations that work when we know exact comparisons are being done, which don't work for case insensitive comparisons (so we have more calls to ToArray on the property names). However, we can try to reduce some of that overhead to bridge the gap a bit.

[ericsampson]

@ahsonkhan are you looking for a design proposal here? Just curious because I see that it's been tagged with the 5.0 milestone, but haven't heard much action since your last post in December. Thanks!!

[ahsonkhan]

are you looking for a design proposal here?

Yes, we at least need a straw man API proposal that facilitates a discussion around the topics I mentioned: https://github.com/dotnet/runtime/issues/31094#issuecomment-567232806

We can then mark this issue as api-ready-for-review and have it reviewed as part of the weekly API review.

From what I recall, there were reservations in the past to expose this global setting as it doesn't compose/scale well as you start to get more complex scenarios where you have multiple dependencies, each with their own "globally" defined options.

Think of an application which overrides the default setting and a library it depends on, which has its own default JsonSerializerOption override, and how can the developer of the app and the developer of the library reason about it in their code (different components, different authors, different timelines).

[andre-ss6]

are you looking for a design proposal here?

Yes, we at least need a straw man API proposal that facilitates a discussion around the topics I mentioned: #31094 (comment)

We can then mark this issue as api-ready-for-review and have it reviewed as part of the weekly API review.

I would rather have a discussion around the points I mentioned earlier before drawing an API proposal, but OK, I'll do it.

[andre-ss6]

@ahsonkhan

Done. I don't know how api review works, but I hope the issue is not closed if this specific API is rejected. I didn't open the issue because I had a specific _API_ in mind, but because I want to have this _feature_. It's also hard not knowing the team's thoughts on the different possible approaches...

[ericsampson]

@ahsonkhan wrote

From what I recall, there were reservations in the past to expose this global setting as it doesn't compose/scale well as you start to get more complex scenarios where you have multiple dependencies, each with their own "globally" defined options.

Think of an application which overrides the default setting and a library it depends on, which has its own default JsonSerializerOption override, and how can the developer of the app and the developer of the library reason about it in their code (different components, different authors, different timelines).

@andre-ss6 thanks for updating your original post with a proposal.
However, I believe that copying Json.NET's design 1-1 is not desirable. My experience is that it's important to have a way for library authors to create a JsonSerializer with their own default settings, not coupled to whatever settings the consumer of the library has set. This has been a pain point with Json.NET's design, and not one I'd like to see repeated.

[andre-ss6]

@andre-ss6 thanks for updating your original post with a proposal.
However, I believe that copying Json.NET's design 1-1 is not desirable. My experience is that it's important to have a way for library authors to create a JsonSerializer with their own default settings, not coupled to whatever settings the consumer of the library has set. This has been a pain point with Json.NET's design, and not one I'd like to see repeated.

@ericsampson I'd love to hear a different suggestion/thoughts on the different approaches. I've been waiting for that since last year.

[SidShetye]

Think of an application which overrides the default setting and a library it depends on, which has its own default JsonSerializerOption override, and how can the developer of the app and the developer of the library reason about it in their code (different components, different authors, different timelines).

Application authors shouldn't suffer due to poor design choices by a library author. Plus that argument essentially boils down to "management of shared resources" - and has been solved before. The obvious advantage to this is greater application design consistency from a single place (e.g. app init) rather than auditing the whole codebase around every http call. Finally, it makes sense to optimize for the common case i.e. JSON serialization options is likely to stay similar rather than change in every instance

So back to the point, either of the two API proposals now in the original post should work.

[ericsampson]

IMO the defaulted options should not cross assembly boundaries; or at least without requiring explicit opt-in to this behavior.

[steveharter]

Updated the main API proposal to:

public static class JsonSerializerOptions
{
    // Can only be set until the first (de)serialization occurs (like other properties here)
    public static Func<JsonSerializerOptions> Default { get; set; }
}

I am assuming it returns null by default meaning a private default options is still used like today.

A simple implementation would be:

    static class MyCustomOptions
    {
        private static JsonSerializerOptions _options;

        // Since we're storing the options in a static variable, just use a static ctor.
        static MyCustomOptions()
        {
            _options = new JsonSerializerOptions
            {
                PropertyNameCaseInsensitive = true,
                PropertyNamingPolicy = JsonNamingPolicy.CamelCase
            };
        }

        public static JsonSerializerOptions MyDefault()
        {
            return _options;
        }
    }

The implementation can be more complicated of course -- you could read the settings from disk, for example, or to change the storage from a static variable to one based on a context variable (like per user or thread).

To register:

JsonSerializerOptions.Default = MyCustomOptions.MyDefault;

[svenso]

I am currently struggling with the default settings on System.Net.Http.Json (there are some different defaults once more).
Because here is going on something, I just wanted to hint, that the new default settings would probably also need to be applied here:
https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/JsonContent.cs

[IanKemp]

I believe that the root of the problem is that JsonSerializer's design is questionable: it is defined as a static class, but has an implicit dependency on a separate stateful class (JsonSerializerOptions) in order to function by default. I understand why this was done - to mimic Newtonsoft.Json as closely as possible, to simplify porting code that uses that API - but the end result is that this state leaks (hence causing the problems noted in this issue). Having the property holding this state private prevents that leak to some extent; changing it to public would effectively open the floodgates.

Therefore, I'd like to suggest a counter-proposal: instead of making JsonSerializer more static, create a non-static wrapper API for it that has an instance of JsonSerializerOptions injected into its constructor, as per the standard DI pattern (in other words, do it right). Something like:

public interface IJsonSerializer
{
    Task<T> DeserializeAsync<T>(Stream utf8Json, JsonSerializerOptions options, CancellationToken cancellationToken);

    // other methods elided for brevity
}

public class DefaultJsonSerializer : IJsonSerializer
{
    private JsonSerializerOptions _defaultOptions;

    public DefaultJsonSerializer(IOptions<JsonSerializerOptions> defaultOptions)
    {
        _defaultOptions = defaultOptions.Value;
    }

    public async Task<T> DeserializeAsync<T>(Stream utf8Json, JsonSerializerOptions options = default, CancellationToken cancellationToken = default)
        => await JsonSerializer.DeserializeAsync<T>(utf8Json, options ?? _defaultOptions, cancellationToken);
}

The default options are configured at startup using the standard DI mechanisms, and your serialization code requests and gets an instance of IJsonSerializer with those options set - you can still override them if absolutely necessary.

This will overcome the issue that @ahsonkhan noted in https://github.com/dotnet/runtime/issues/31094#issuecomment-563075492 - for example in an ASP.NET Core context, the options that it sets for its defaults, plus any user overrides set via AddJsonOptions(), will now be implicitly used by IJsonSerializer, and everything Just Works.

Of course, nothing prevents anyone from applying this suggestion, right now. :)