ENH: Use RapidJSON instead of JsonCPP for performance improvement #646
Conversation
RapidJSON is mganitudes faster than JsonCPP (https://github.com/miloyip/nativejson-benchmark). This is important, because when terminologies (rather large json documents) are parsed/searched/modified then each operation may take hundreds of milliseconds with JsonCPP. Multiple calls easily accumulate to user-perceivable delays (magnitude of seconds). By keep using JsonCPP we would constantly have to optimize number of logic calls to make sure only json access is minimized. Also, JsonCPP was unbearably slow in debug mode in Windows (simple operations took tens of seconds). To minimize development time and optimize speed we switch to RapidJSON instead. RapidJSON's advantages compared to jsoncpp (https://github.com/miloyip/nativejson-benchmark): * Magnitudes faster, also, its API facilitates writing efficient application code * CMake-ified * Header-only (no need to install DLLs, ...) * More conform to json standard * Library is much smaller (31k vs 243k)
|
@jcfr Could you please check if CMake files are updated correctly? I haven't added an install step, as it's header-only. |
|
Thanks! I just started a clean build, will let you know how it goes. |
|
Nice! Why didn't we use RapidJSON from the start? @vovythevov |
|
@dzenanz: We didn't really need to profile the json use before. We just used what was already in place with the SlicerExecutionModel and the parameter serializer. |
|
It was added to SEM and ParameterSerializer only a few weeks before it got into Slicer core. I guess the reason for the choice was that VTK used the same thing and back in the day when they evaluated the options JsonCpp was one of the best. |
jsoncpp cannot be removed until at the very least SlicerExecutionModel dependency on jsoncpp is removed. dcmqi (and indirectly - QuantitativeReporting) both depend on jsoncpp. In any case, this PR does not remove jsoncpp. |
|
I think the title of this PR is confusing - it should be "Use RapidJSON for segment terminology instead of JsonCPP for performance improvement". The current title creates the impression that jsoncpp is completely removed. |
|
I did a quick test, and it seems to work fine. It is super fast in debug mode, as opposed to before when we needed to wait several seconds to the query to go through (in release mode it was much faster, but slow enough to make certain operations choppy). Now that it's fast even in debug mode, it's safe to say that release mode will be faster too. |
|
It's good to leave both options in place for now, but if we have a strong preference for rapidjson we should deprecate jsoncpp. |
RapidJSON does not generate CMake use files - add to Slicer_EXTERNAL_PROJECTS_NO_USEFILE_CONFIG list.
|
If I don't get any more feedback on this then I'll integrate this tomorrow morning. |
|
Thanks Jc. |
RapidJSON is mganitudes faster than JsonCPP (https://github.com/miloyip/nativejson-benchmark).
This is important, because when terminologies (rather large json documents) are parsed/searched/modified then each operation may take hundreds of milliseconds
with JsonCPP. Multiple calls easily accumulate to user-perceivable delays (magnitude of seconds). By keep using JsonCPP we would constantly have to
optimize number of logic calls to make sure only json access is minimized. Also, JsonCPP was unbearably slow in debug mode in Windows (simple operations took
tens of seconds).
To minimize development time and optimize speed we switch to RapidJSON instead. RapidJSON's advantages compared to jsoncpp (https://github.com/miloyip/nativejson-benchmark):