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

Improve JSON (de)serialization performance with go-json #559

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

foxdalas
Copy link
Contributor

@foxdalas foxdalas commented Nov 24, 2023

Description

More effective JSON serialization and deserialization.

Motivation and Context

We have more than 20k RPS on our Flagr installation, and we are trying to find a way to improve performance.

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ghost
Copy link

ghost commented Nov 24, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@foxdalas
Copy link
Contributor Author

foxdalas commented Nov 28, 2023

Before

BenchmarkEvalFlag-12               71962             17172 ns/op            1720 B/op         32 allocs/op
BenchmarkEvalFlagsByTags-12        74755             16713 ns/op            1946 B/op         36 allocs/op

After

BenchmarkEvalFlag-12               72667             17019 ns/op            1721 B/op         32 allocs/op
BenchmarkEvalFlagsByTags-12        74432             16545 ns/op            1945 B/op         36 allocs/op

@marceloboeira
Copy link
Member

marceloboeira commented Dec 1, 2023

@foxdalas thanks for contribution. It seems some integration tests are still failing, would you mind checking those?

@foxdalas
Copy link
Contributor Author

foxdalas commented Dec 1, 2023

@foxdalas thanks for contribution. It seems some integration tests are still failing, would you make checking those?

Yes, I will

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fcc74ae) 81.07% compared to head (a13e105) 81.18%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #559      +/-   ##
==========================================
+ Coverage   81.07%   81.18%   +0.11%     
==========================================
  Files          28       28              
  Lines        2256     2270      +14     
==========================================
+ Hits         1829     1843      +14     
  Misses        337      337              
  Partials       90       90              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marceloboeira
Copy link
Member

marceloboeira commented Dec 4, 2023

@foxdalas thanks for the contribution!

We have more than 20k RPS on our Flagr installation, and we are trying to find a way to improve performance.

I'm wondering, would you be able to deploy this branch to your instance and share some performance gains? I think those are not so easy to see in the local tests here.

@foxdalas
Copy link
Contributor Author

foxdalas commented Dec 4, 2023

@foxdalas thanks for the contribution!

We have more than 20k RPS on our Flagr installation, and we are trying to find a way to improve performance.

I'm wondering, would you be able to deploy this branch to your instance and share some performance gains? I think those are not so easy to see in the local tests here.

I have planned test to tomorrow. I will back day after tomorrow

@foxdalas foxdalas changed the title add go-json WIP: add go-json Dec 4, 2023
@foxdalas
Copy link
Contributor Author

foxdalas commented Dec 7, 2023

@marceloboeira The same environment, configuration and requests rate.

total profit ~25%

Screenshot 2566-12-07 at 15 59 39 Screenshot 2566-12-07 at 15 59 03 Screenshot 2566-12-07 at 15 59 16 Screenshot 2566-12-07 at 15 59 33

@foxdalas foxdalas changed the title WIP: add go-json add go-json Dec 7, 2023
@marceloboeira marceloboeira changed the title add go-json Improve JSON (de)serialization performaaance with go-json Dec 7, 2023
@marceloboeira marceloboeira changed the title Improve JSON (de)serialization performaaance with go-json Improve JSON (de)serialization performance with go-json Dec 7, 2023
@marceloboeira
Copy link
Member

@foxdalas awesome!

@marceloboeira marceloboeira merged commit 3f9f613 into openflagr:main Dec 7, 2023
9 checks passed
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