plone / guillotina

Python AsyncIO data API to manage billions of resources
https://guillotina.readthedocs.io/en/latest/
Other
187 stars 51 forks source link

Use orjson instead of json/ujson #940

Closed masipcat closed 4 years ago

masipcat commented 4 years ago

I'm still testing and doing stress benchmarks to ensure there is a performance gain

codecov-io commented 4 years ago

Codecov Report

Merging #940 into master will decrease coverage by 0.1%. The diff coverage is 73.9%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #940     +/-   ##
========================================
- Coverage    94.5%   94.5%   -0.0%     
========================================
  Files         333     333             
  Lines       29550   29549      -1     
========================================
- Hits        27909   27903      -6     
- Misses       1641    1646      +5     
Impacted Files Coverage Δ
guillotina/renderers.py 82.9% <63.0%> (-3.9%) :arrow_down:
guillotina/request.py 86.0% <87.5%> (-0.6%) :arrow_down:
guillotina/contrib/redis/dm.py 100.0% <100.0%> (ø)
guillotina/exceptions.py 87.9% <100.0%> (ø)
guillotina/tests/test_ws.py 100.0% <100.0%> (ø)
masipcat commented 4 years ago

Benchmarks

Using molotov:

  json (reqps) orjson (reqps) diff
#1 5064 5661  
#2 5423 5587  
#3 5107 5401  
       
Mean 5198 5549.7 +6.77%

Using molotov:

  json (reqps) orjson (reqps) diff
#1 5095 5786  
#2 5148 5986  
#3 4816 5925  
       
Mean 5019.7 5899 +17.52%

So, we have a small improvement (+6%) for small json data and a relatively big improvement for big json data (+17%).

vangheem commented 4 years ago

Sweet! It looks fine to me. Do we want b/w compat GuillotinaJSONEncoder still?

masipcat commented 4 years ago

Sweet! It looks fine to me. Do we want b/w compat GuillotinaJSONEncoder still?

I think we can add it. Is just 3 lines...

masipcat commented 4 years ago

Ready for review!