ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.31k stars 1.63k forks source link

Remove the `Newtonsoft.Json` package and migrate to `System.Text.Json` #2125

Open EngRajabi opened 1 month ago

EngRajabi commented 1 month ago

In this request, we deleted the newtonsoft package and migrated to text json system. The reason for the changes

@MohammadAminPourmoradian helped me with this

BenchmarkDotNet v0.13.11, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3) Intel Core i7-10870H CPU 2.20GHz, 1 CPU, 16 logical and 8 physical cores .NET SDK 8.0.303 [Host] : .NET 6.0.32 (6.0.3224.31407), X64 RyuJIT AVX2 [AttachedDebugger] .NET 8.0 : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2

Job=.NET 8.0 Runtime=.NET 8.0

Method Count Mean Error StdDev Median Op/s Gen0 Gen1 Gen2 Allocated
MicrosoftDeserializeBigData 1000 856.3 us 53.98 us 157.47 us 797.1 us 1,167.8 39.0625 13.6719 - 328.78 KB
NewtonsoftDeserializeBigData 1000 1,137.2 us 18.74 us 17.53 us 1,132.8 us 879.4 54.6875 17.5781 - 457.94 KB
MicrosoftSerializeBigData 1000 646.4 us 12.72 us 20.90 us 645.7 us 1,546.9 110.3516 110.3516 110.3516 350.02 KB
NewtonsoftSerializeBigData 1000 1,033.4 us 19.37 us 42.53 us 1,022.8 us 967.7 109.3750 109.3750 109.3750 837.82 KB
raman-m commented 1 month ago

build - Skipped This is very strange... We need to fix the PR build

raman-m commented 1 month ago

Mohsen, if you don't mind, I will rebase the branch once again... I hope to fix the build.

raman-m commented 1 month ago

build - Success

Very well❕

raman-m commented 1 month ago

@EngRajabi @MohammadAminPourmoradian Mohsen and Mohammad, congratulations! The PR has entered the official code review stage. The build is green and stable now. I've made some on-the-fly code review adjustments, including superficial changes. The pull request is now well-prepared for an easy review. Could you consider working on the suggestions from my code review plz?

@ggnaegi @RaynaldM Hello, I anticipate your thorough code review due to the significant and important upgrade in the Core involving the replacement of the JSON parsing library. I would value your consideration of my proposal to redesign the current draft solution, as I foresee potential issues.

EngRajabi commented 1 month ago

@raman-m requested changes on Jul 22, 2024

You gave a good suggestion, but is it really that much work? Microsoft package system text json is executed on net 6 7 8. Old versions are not affected because json behavior has not changed. It can be done by separating a serializer layer, but is it really necessary? How often does it change?

ggnaegi commented 1 month ago

@raman-m let's check this...