[3.2] Implement JSON Snapshot Reader #732
Conversation
libraries/chain/snapshot.cpp
Outdated
| ,cur_row(0) | ||
| { | ||
| std::stringstream my_buffer; | ||
| my_buffer << snapshot.rdbuf(); |
There was a problem hiding this comment.
This looks like it reads the entire file in to memory? How much memory does this take for something like an EOS or WAX snapshot? It almost seems like that's going to be impractical.
|
|
||
| EOS_ASSERT( options.count( "genesis-timestamp" ) == 0, | ||
| plugin_config_exception, | ||
| "--snapshot is incompatible with --genesis-timestamp as the snapshot contains genesis information"); |
There was a problem hiding this comment.
--json-snapshot not --snapshot. But more generally this looks like 40 lines copy pasted (almost?) verbatim. Is there a better way?
…erge snapshot and jsonsnapshot code handling
|
Have we (manually) tested that we can load something like a WAX snapshot in JSON format with this? |
I doubled my WSL max storage yesterday and attempted to run WAX but the snapshot-to-json ran my VM out of space at this row: I can allocate some more space as well as delete some stuff to try and fit all of wax's JSON Snapshot tonight but I have no idea how much space the full thing is going to take to create. |
| @@ -5,6 +5,7 @@ | |||
| #include <fc/variant_object.hpp> | |||
| #include <boost/core/demangle.hpp> | |||
| #include <ostream> | |||
| #include "../rapidjson/include/rapidjson/document.h" | |||
There was a problem hiding this comment.
it really should be added to cmake as an INTERFACE library and properly set up so only need to #include <rapidjson/document.h>
|
I think only one version of rapidjson should be in the global namespace so we avoid any ODR wonkiness . Right now, I believe abieos includes it in the global namespace too. |
…d of adding a new option. Improved memory usage so current WAX snapshot can be supported. Modified the JSON output so that table rows are an array which reduces file size and memory usage. Put rapidjson inside an eosio_rapidjson namespace to avoid ODR issues. Change CMakeLists to not compile rapidjson but only include it as it is a header only library.
| @@ -131,6 +131,7 @@ target_link_libraries( eosio_chain PUBLIC fc chainbase Logging IR WAST WASM Runt | |||
| target_include_directories( eosio_chain | |||
| PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/include" "${CMAKE_CURRENT_BINARY_DIR}/include" | |||
| "${CMAKE_CURRENT_SOURCE_DIR}/../wasm-jit/Include" | |||
| "${CMAKE_CURRENT_SOURCE_DIR}/../rapidjson/include" | |||
There was a problem hiding this comment.
This really needs to be an INTERFACE library
There was a problem hiding this comment.
Can you provide an example of what that would look like. I didn't see an example in our code.
There was a problem hiding this comment.
something like (I'm just winging this so may not be fully correct)
add_library(rapidjson INTERFACE)
target_include_directories(rapidjson INTERFACE rapidjson/include)
target_compile_definitions(rapidjson INTERFACE -DRAPIDJSON_NAMESPACE=eosio_rapidjson)
then wherever you want to use it just
target_link_libraries(eosio_chain rapidjson)
There was a problem hiding this comment.
I'm not sure if making the RAPIDJSON_NAMESPACE thing part of the interface is a good idea or not though.
|
closing due to migration to leap AntelopeIO/leap#25 |
#710 ports a way to convert a snapshot to json. This PR enhances the
--snapshotoption to support reading a snapshot in JSON format if the snapshot has.jsonextension.Resolves: #771