ibireme / yyjson

The fastest JSON library in C
https://ibireme.github.io/yyjson/doc/doxygen/html/
MIT License
3.04k stars 262 forks source link

Add Mutable yyjson val Detach Feature #111

Closed faultaddr closed 1 year ago

faultaddr commented 1 year ago

In order to be able to manage the memory allocated by mut_val separately, I have made some changes to make it happen.

struct yyjson_mut_val {
    uint64_t tag; /**< type, subtype and length */
    yyjson_val_uni uni; /**< payload */
    yyjson_mut_val *next; /**< the next value in circular linked list */
    yyjson_val_chunk *val_chunk;  // val memory chunk address
    yyjson_str_chunk *str_chunk;  // str memory chunk address
};

All changes are related to it. #106

codecov[bot] commented 1 year ago

Codecov Report

Merging #111 (451398c) into master (77cce46) will decrease coverage by 0.80%. The diff coverage is 71.60%.

@@            Coverage Diff             @@
##           master     #111      +/-   ##
==========================================
- Coverage   98.49%   97.69%   -0.80%     
==========================================
  Files           2        2              
  Lines        2452     2521      +69     
==========================================
+ Hits         2415     2463      +48     
- Misses         37       58      +21     
Impacted Files Coverage Δ
src/yyjson.h 86.74% <50.00%> (-0.32%) :arrow_down:
src/yyjson.c 98.07% <72.72%> (-0.83%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ibireme commented 1 year ago

The change looks good, but this library is more focused on performance, so it doesn’t seem like a good fit.

I ran some benchmarks and found a significant performance drop. For example, the write speed of this benchmark decreased by 36% from 653MB/s to 417MB/s.

The main reason is that this PR disabled the memory pool in two pool_grow() functions, which causes frequent memory allocation when creating yyjson_mut_val, leading to a significant performance decrease. Additionally, the new fields added to the yyjson_mut_val struct will also result in greater memory consumption.

faultaddr commented 1 year ago

You're right!But I still have some questions. LOL,According to my guess, the write/read test only conduct on yyjson_doc rather than yyjson_mut_doc. For yyjson_doc , there is no need to use the memory pool whether reading or writing.

However, Disabling the memory pool will definitely cause a performance loss. The yyjson_mut_doc will be affected

ibireme commented 1 year ago

In the link above, the benchmark does evaluate the performance of yyjson_mut_doc (create JSON from scratch and serialize it). For that task, the memory pool has a significant impact on performance. A bigger memory pool means faster performance, but no memory pool means slower performance.

faultaddr commented 1 year ago

Roger that! The Memory Pool is significant for writing. I just want to make the yyjson more flexable, but my proposal might be a bad choice, thank you anyway!