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

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

Closed
wants to merge 27 commits into from

Conversation

EngRajabi
Copy link
Contributor

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

  • Newtonsoft package is no longer developed
  • System text json package was developed by Microsoft itself and has many changes and improvements day by day
  • System text json has a much better performance
  • It consumes much less RAM
    The changes given include the removal of Newtonsoft. A benchmark has also been added for this

@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 raman-m changed the title Removing the newtonsoft package and migrating to system text json Remove the Newtonsoft.Json package and migrate to System.Text.Json Jul 19, 2024
@raman-m
Copy link
Member

raman-m commented Jul 19, 2024

Mohsen, thank you for the nice PR. However, I've noticed some instability in the CircleCI jobs on the feature branch, which seems to stem from issues in the develop branch. It's important to adhere to our development process by not creating feature branches from a broken develop branch.

  1. Could you align your develop branch with the ThreeMammals/Ocelot:develop branch? Your develop branch is currently 13 commits ahead of the ThreeMammals/Ocelot:develop, but there should be no differences. Your develop branch contains numerous Merge branch 'ThreeMammals:develop' into develop commits, which are causing instability in our CircleCI builds. The quickest solution would be to delete the develop branch and then recreate it, ensuring it's fully synchronized with the head repository. If the branch is protected, please unprotect it, possibly making the main branch protected instead. If this process is too complex, consider adding me as a collaborator to your forked repository, and I will address the issue.

  2. After aligning the develop branch with the head repository, it's necessary to rebase your feature branch due to several issues:

    • It was created from a defective develop branch.
    • It includes numerous false changes which emerged after resolving conflicts, making it difficult to discern the actual changes amidst the spurious ones.

Once these issues are resolved, we can commence the discussion on the feature.

@EngRajabi
Copy link
Contributor Author

EngRajabi commented Jul 19, 2024

@raman-m #2125

@EngRajabi EngRajabi closed this Jul 19, 2024
@raman-m
Copy link
Member

raman-m commented Jul 22, 2024

Thank you, Mohsen!
I have reviewed your new pull request. Your develop branch is now identical to the head repository (no differences), which is correct.

This branch is up to date with ThreeMammals/Ocelot:develop.

This is indeed correct!

@EngRajabi EngRajabi deleted the feat_metric branch July 22, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants