From 30fc616412bfaf3b62a89fd1de288feba52e0706 Mon Sep 17 00:00:00 2001 From: Qrox Date: Sat, 26 Dec 2020 16:24:50 +0800 Subject: [PATCH 1/2] Show source location of deferred json in debug messages --- src/generic_factory.h | 3 ++- src/init.cpp | 33 ++++++++++++++++++---------- src/init.h | 6 ++++-- src/item_factory.cpp | 3 ++- src/json.cpp | 16 ++++++++++++++ src/json.h | 21 +++++++++++++++--- src/mapgen.cpp | 45 +++++++++++++++++++++++---------------- src/mapgen.h | 10 ++++----- src/missiondef.cpp | 3 ++- src/recipe_dictionary.cpp | 3 ++- src/veh_type.cpp | 3 ++- 11 files changed, 102 insertions(+), 44 deletions(-) diff --git a/src/generic_factory.h b/src/generic_factory.h index f484d744fc6fe..08aa04693a3d8 100644 --- a/src/generic_factory.h +++ b/src/generic_factory.h @@ -221,7 +221,8 @@ class generic_factory def = ab->second; } else { def.was_loaded = false; - deferred.emplace_back( jo.str(), src ); + deferred.emplace_back( jo.get_source_location(), src ); + jo.allow_omitted_members(); return false; } } diff --git a/src/init.cpp b/src/init.cpp index 053287d592000..1534efc879aa9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -125,24 +125,35 @@ void DynamicDataLoader::load_deferred( deferred_json &data ) const size_t n = data.size(); auto it = data.begin(); for( size_t idx = 0; idx != n; ++idx ) { - try { - std::istringstream str( it->first ); - JsonIn jsin( str ); - JsonObject jo = jsin.get_object(); - load_object( jo, it->second ); - } catch( const std::exception &err ) { - debugmsg( "Error loading data from json: %s", err.what() ); + if( !it->first.path ) { + debugmsg( "JSON source location has null path, data may load incorrectly" ); + } else { + try { + std::ifstream str( *it->first.path, std::ifstream::in | std::ifstream::binary ); + JsonIn jsin( str, it->first ); + JsonObject jo = jsin.get_object(); + load_object( jo, it->second ); + } catch( const JsonError &err ) { + debugmsg( "(json-error)\n%s", err.what() ); + } } ++it; } data.erase( data.begin(), it ); if( data.size() == n ) { - std::string discarded; for( const auto &elem : data ) { - discarded += elem.first; + if( !elem.first.path ) { + debugmsg( "JSON source location has null path when reporting circular dependency" ); + } else { + try { + std::ifstream str( *elem.first.path, std::ifstream::in | std::ifstream::binary ); + JsonIn jsin( str, elem.first ); + jsin.error( "JSON contains circular dependency, this object is discarded" ); + } catch( const JsonError &err ) { + debugmsg( "(json-error)\n%s", err.what() ); + } + } } - debugmsg( "JSON contains circular dependency. Discarded %i objects:\n%s", - data.size(), discarded ); data.clear(); return; // made no progress on this cycle so abort } diff --git a/src/init.h b/src/init.h index 6c83ae1977e34..51457f2c92e3f 100644 --- a/src/init.h +++ b/src/init.h @@ -9,6 +9,8 @@ #include #include +#include "json.h" + class loading_ui; class JsonObject; class JsonIn; @@ -59,9 +61,9 @@ class DynamicDataLoader /** * JSON data dependent upon as-yet unparsed definitions - * first: JSON data, second: source identifier + * first: JSON source location, second: source identifier */ - using deferred_json = std::list>; + using deferred_json = std::list>; private: bool finalized = false; diff --git a/src/item_factory.cpp b/src/item_factory.cpp index d744ea43d830e..7bcf2b71a2513 100644 --- a/src/item_factory.cpp +++ b/src/item_factory.cpp @@ -1611,7 +1611,8 @@ bool Item_factory::load_definition( const JsonObject &jo, const std::string &src return true; } - deferred.emplace_back( jo.str(), src ); + deferred.emplace_back( jo.get_source_location(), src ); + jo.allow_omitted_members(); return false; } diff --git a/src/json.cpp b/src/json.cpp index cc5e068e338fe..09fd46e3e7e05 100644 --- a/src/json.cpp +++ b/src/json.cpp @@ -252,6 +252,21 @@ JsonIn *JsonObject::get_raw( const std::string &name ) const return jsin; } +json_source_location JsonObject::get_source_location() const +{ + if( !jsin ) { + throw JsonError( "JsonObject::get_source_location called when stream is null" ); + } + json_source_location loc; + loc.path = jsin->get_path(); + if( !loc.path ) { + jsin->seek( start ); + jsin->error( "JsonObject::get_source_location called but the path is unknown" ); + } + loc.offset = start; + return loc; +} + /* returning values by name */ bool JsonObject::get_bool( const std::string &name ) const @@ -1629,6 +1644,7 @@ bool JsonIn::read( JsonDeserializer &j, bool throw_on_error ) // WARNING: for occasional use only. std::string JsonIn::line_number( int offset_modifier ) { + const std::string &name = path ? *path : ""; if( stream && stream->eof() ) { return name + ":EOF"; } else if( !stream || stream->fail() ) { diff --git a/src/json.h b/src/json.h index 05f623d7ee8f7..53efbf1fb31db 100644 --- a/src/json.h +++ b/src/json.h @@ -17,6 +17,7 @@ #include "colony.h" #include "enum_conversions.h" +#include "memory_fast.h" #include "string_id.h" /* Cataclysm-DDA homegrown JSON tools @@ -93,6 +94,11 @@ struct number_sci_notation { int64_t exp = 0; }; +struct json_source_location { + shared_ptr_fast path; + int offset = 0; +}; + /* JsonIn * ====== * @@ -177,8 +183,7 @@ class JsonIn { private: std::istream *stream; - // Used for error message and thus intentionally untranslated - std::string name = ""; + shared_ptr_fast path; bool ate_separator = false; void skip_separator(); @@ -187,10 +192,19 @@ class JsonIn public: JsonIn( std::istream &s ) : stream( &s ) {} - JsonIn( std::istream &s, const std::string &name ) : stream( &s ), name( name ) {} + JsonIn( std::istream &s, const std::string &path ) + : stream( &s ), path( make_shared_fast( path ) ) {} + JsonIn( std::istream &s, const json_source_location &loc ) + : stream( &s ), path( loc.path ) { + seek( loc.offset ); + } JsonIn( const JsonIn & ) = delete; JsonIn &operator=( const JsonIn & ) = delete; + shared_ptr_fast get_path() const { + return path; + } + bool get_ate_separator() { return ate_separator; } @@ -895,6 +909,7 @@ class JsonObject // seek to a value and return a pointer to the JsonIn (member must exist) JsonIn *get_raw( const std::string &name ) const; JsonValue get_member( const std::string &name ) const; + json_source_location get_source_location() const; // values by name // variants with no fallback throw an error if the name is not found. diff --git a/src/mapgen.cpp b/src/mapgen.cpp index cac0efc4248b8..59b6c40443337 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -4,12 +4,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include @@ -443,9 +443,11 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, const p jio.throw_error( "function does not exist", "name" ); } } else if( mgtype == "json" ) { - const std::string jstr = jio.get_object( "object" ).str(); + JsonObject jo = jio.get_object( "object" ); + const json_source_location jsrc = jo.get_source_location(); + jo.allow_omitted_members(); ret = std::make_shared( - jstr, mgweight, "mapgen " + id_base, offset ); + jsrc, mgweight, "mapgen " + id_base, offset ); oter_mapgen.add( id_base, ret ); } else { jio.throw_error( R"(invalid value: must be "builtin" or "json")", "method" ); @@ -460,9 +462,10 @@ static void load_nested_mapgen( const JsonObject &jio, const std::string &id_bas if( jio.has_object( "object" ) ) { int weight = jio.get_int( "weight", 1000 ); JsonObject jo = jio.get_object( "object" ); - std::string jstr = jo.str(); + const json_source_location jsrc = jo.get_source_location(); + jo.allow_omitted_members(); nested_mapgen[id_base].add( - std::make_shared( jstr, "nested mapgen " + id_base ), + std::make_shared( jsrc, "nested mapgen " + id_base ), weight ); } else { debugmsg( "Nested mapgen: Invalid mapgen function (missing \"object\" object)", id_base.c_str() ); @@ -479,10 +482,11 @@ static void load_update_mapgen( const JsonObject &jio, const std::string &id_bas if( mgtype == "json" ) { if( jio.has_object( "object" ) ) { JsonObject jo = jio.get_object( "object" ); - std::string jstr = jo.str(); + const json_source_location jsrc = jo.get_source_location(); + jo.allow_omitted_members(); update_mapgen[id_base].push_back( std::make_unique( - jstr, "update mapgen " + id_base ) ); + jsrc, "update mapgen " + id_base ) ); } else { debugmsg( "Update mapgen: Invalid mapgen function (missing \"object\" object)", id_base.c_str() ); @@ -588,8 +592,8 @@ bool mapgen_function_json_base::check_inbounds( const jmapgen_int &x, const jmap } mapgen_function_json_base::mapgen_function_json_base( - const std::string &s, const std::string &context ) - : jdata( s ) + const json_source_location &jsrcloc, const std::string &context ) + : jsrcloc( jsrcloc ) , context_( context ) , do_format( false ) , is_ready( false ) @@ -600,10 +604,10 @@ mapgen_function_json_base::mapgen_function_json_base( mapgen_function_json_base::~mapgen_function_json_base() = default; -mapgen_function_json::mapgen_function_json( const std::string &s, const int w, +mapgen_function_json::mapgen_function_json( const json_source_location &jsrcloc, const int w, const std::string &context, const point &grid_offset ) : mapgen_function( w ) - , mapgen_function_json_base( s, context ) + , mapgen_function_json_base( jsrcloc, context ) , fill_ter( t_null ) , rotation( 0 ) { @@ -613,8 +617,8 @@ mapgen_function_json::mapgen_function_json( const std::string &s, const int w, } mapgen_function_json_nested::mapgen_function_json_nested( - const std::string &s, const std::string &context ) - : mapgen_function_json_base( s, context ) + const json_source_location &jsrcloc, const std::string &context ) + : mapgen_function_json_base( jsrcloc, context ) , rotation( 0 ) { } @@ -2431,8 +2435,12 @@ void mapgen_function_json_base::setup_common() if( is_ready ) { return; } - std::istringstream iss( jdata ); - JsonIn jsin( iss ); + if( !jsrcloc.path ) { + debugmsg( "null json source location path" ); + return; + } + std::ifstream ifs( *jsrcloc.path, std::ifstream::in | std::ifstream::binary ); + JsonIn jsin( ifs, jsrcloc ); JsonObject jo = jsin.get_object(); mapgen_defer::defer = false; if( !setup_common( jo ) ) { @@ -6484,8 +6492,8 @@ void add_corpse( map *m, const point &p ) //////////////////// mapgen update update_mapgen_function_json::update_mapgen_function_json( - const std::string &s, const std::string &context ) : - mapgen_function_json_base( s, context ) + const json_source_location &jsrcloc, const std::string &context ) : + mapgen_function_json_base( jsrcloc, context ) { } @@ -6578,7 +6586,8 @@ mapgen_update_func add_mapgen_update_func( const JsonObject &jo, bool &defer ) return update_function; } - update_mapgen_function_json json_data( "", "unknown object in add_mapgen_update_func" ); + update_mapgen_function_json json_data( json_source_location {}, + "unknown object in add_mapgen_update_func" ); mapgen_defer::defer = defer; if( !json_data.setup_update( jo ) ) { const auto null_function = []( const tripoint_abs_omt &, mission * ) { diff --git a/src/mapgen.h b/src/mapgen.h index a04898055f7ad..4e93be06d0a0a 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -319,11 +319,11 @@ class mapgen_function_json_base bool has_vehicle_collision( const mapgendata &dat, const point &offset ) const; private: - std::string jdata; + json_source_location jsrcloc; std::string context_; protected: - mapgen_function_json_base( const std::string &s, const std::string &context ); + mapgen_function_json_base( const json_source_location &jsrcloc, const std::string &context ); virtual ~mapgen_function_json_base(); void setup_common(); @@ -354,7 +354,7 @@ class mapgen_function_json : public mapgen_function_json_base, public virtual ma void setup() override; void check() const override; void generate( mapgendata & ) override; - mapgen_function_json( const std::string &s, int w, const std::string &context, + mapgen_function_json( const json_source_location &jsrcloc, int w, const std::string &context, const point &grid_offset = point_zero ); ~mapgen_function_json() override = default; @@ -371,7 +371,7 @@ class mapgen_function_json : public mapgen_function_json_base, public virtual ma class update_mapgen_function_json : public mapgen_function_json_base { public: - update_mapgen_function_json( const std::string &s, const std::string &context ); + update_mapgen_function_json( const json_source_location &jsrcloc, const std::string &context ); ~update_mapgen_function_json() override = default; void setup(); @@ -392,7 +392,7 @@ class mapgen_function_json_nested : public mapgen_function_json_base public: void setup(); void check() const; - mapgen_function_json_nested( const std::string &s, const std::string &context ); + mapgen_function_json_nested( const json_source_location &jsrcloc, const std::string &context ); ~mapgen_function_json_nested() override = default; void nest( const mapgendata &dat, const point &offset ) const; diff --git a/src/missiondef.cpp b/src/missiondef.cpp index 97ba3d7012165..9c6a7fe56cbed 100644 --- a/src/missiondef.cpp +++ b/src/missiondef.cpp @@ -287,7 +287,8 @@ void mission_type::load( const JsonObject &jo, const std::string &src ) } else if( jo.has_member( phase ) ) { JsonObject j_start = jo.get_object( phase ); if( !parse_funcs( j_start, phase_func ) ) { - deferred.emplace_back( jo.str(), src ); + deferred.emplace_back( jo.get_source_location(), src ); + jo.allow_omitted_members(); j_start.allow_omitted_members(); return false; } diff --git a/src/recipe_dictionary.cpp b/src/recipe_dictionary.cpp index 6c00036b359e5..2d0c096ddf85a 100644 --- a/src/recipe_dictionary.cpp +++ b/src/recipe_dictionary.cpp @@ -324,7 +324,8 @@ recipe &recipe_dictionary::load( const JsonObject &jo, const std::string &src, if( jo.has_string( "copy-from" ) ) { auto base = recipe_id( jo.get_string( "copy-from" ) ); if( !out.count( base ) ) { - deferred.emplace_back( jo.str(), src ); + deferred.emplace_back( jo.get_source_location(), src ); + jo.allow_omitted_members(); return null_recipe; } r = out[ base ]; diff --git a/src/veh_type.cpp b/src/veh_type.cpp index ce6166cc80fef..fefeda869660e 100644 --- a/src/veh_type.cpp +++ b/src/veh_type.cpp @@ -357,7 +357,8 @@ void vpart_info::load( const JsonObject &jo, const std::string &src ) } def.categories = ab->second.categories; } else { - deferred.emplace_back( jo.str(), src ); + deferred.emplace_back( jo.get_source_location(), src ); + jo.allow_omitted_members(); return; } } From 4c9eb7b64caf334ade21e95cbe39727299131805 Mon Sep 17 00:00:00 2001 From: Qrox Date: Sun, 27 Dec 2020 01:50:06 +0800 Subject: [PATCH 2/2] Cache deferred json input stream --- src/init.cpp | 34 ++++++++++++++++++++++++++++++---- src/init.h | 12 ++++++++++++ src/lru_cache.cpp | 4 ++++ src/mapgen.cpp | 6 +++--- 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 1534efc879aa9..986cb4a1ad508 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -44,6 +44,7 @@ #include "item_factory.h" #include "json.h" #include "loading_ui.h" +#include "lru_cache.h" #include "magic.h" #include "magic_enchantment.h" #include "magic_ter_furn_transform.h" @@ -119,6 +120,30 @@ void DynamicDataLoader::load_object( const JsonObject &jo, const std::string &sr it->second( jo, src, base_path, full_path ); } +struct DynamicDataLoader::cached_streams { + lru_cache> cache; +}; + +shared_ptr_fast DynamicDataLoader::get_cached_stream( const std::string &path ) +{ + cata_assert( !finalized && + "Cannot open data file after finalization." ); + if( !stream_cache ) { + stream_cache = std::make_unique(); + } + shared_ptr_fast cached = stream_cache->cache.get( path, nullptr ); + // Create a new stream if the file is not opened yet, or if some code is still + // using the previous stream (in such case, `cached` and `stream_cache` have + // two references to the stream, hence the test for > 2). + if( !cached ) { + cached = make_shared_fast( read_entire_file( path ) ); + } else if( cached.use_count() > 2 ) { + cached = make_shared_fast( cached->str() ); + } + stream_cache->cache.insert( 8, path, cached ); + return cached; +} + void DynamicDataLoader::load_deferred( deferred_json &data ) { while( !data.empty() ) { @@ -129,8 +154,8 @@ void DynamicDataLoader::load_deferred( deferred_json &data ) debugmsg( "JSON source location has null path, data may load incorrectly" ); } else { try { - std::ifstream str( *it->first.path, std::ifstream::in | std::ifstream::binary ); - JsonIn jsin( str, it->first ); + shared_ptr_fast stream = get_cached_stream( *it->first.path ); + JsonIn jsin( *stream, it->first ); JsonObject jo = jsin.get_object(); load_object( jo, it->second ); } catch( const JsonError &err ) { @@ -146,8 +171,8 @@ void DynamicDataLoader::load_deferred( deferred_json &data ) debugmsg( "JSON source location has null path when reporting circular dependency" ); } else { try { - std::ifstream str( *elem.first.path, std::ifstream::in | std::ifstream::binary ); - JsonIn jsin( str, elem.first ); + shared_ptr_fast stream = get_cached_stream( *it->first.path ); + JsonIn jsin( *stream, elem.first ); jsin.error( "JSON contains circular dependency, this object is discarded" ); } catch( const JsonError &err ) { debugmsg( "(json-error)\n%s", err.what() ); @@ -651,6 +676,7 @@ void DynamicDataLoader::finalize_loaded_data( loading_ui &ui ) } check_consistency( ui ); + stream_cache.reset(); finalized = true; } diff --git a/src/init.h b/src/init.h index 51457f2c92e3f..2001bb68bcea6 100644 --- a/src/init.h +++ b/src/init.h @@ -10,6 +10,7 @@ #include #include "json.h" +#include "memory_fast.h" class loading_ui; class JsonObject; @@ -68,6 +69,9 @@ class DynamicDataLoader private: bool finalized = false; + struct cached_streams; + std::unique_ptr stream_cache; + protected: /** * Maps the type string (coming from json) to the @@ -163,6 +167,14 @@ class DynamicDataLoader bool is_data_finalized() const { return finalized; } + + /** + * Get a possibly cached stream for deferred data loading. If the cached + * stream is still in use by outside code, this returns a new stream to + * avoid conflict of stream cursor. The stream cursor is not reset if a + * cached stream is returned. + */ + shared_ptr_fast get_cached_stream( const std::string &path ); }; #endif // CATA_SRC_INIT_H diff --git a/src/lru_cache.cpp b/src/lru_cache.cpp index c04582d0e77c5..6038ca7ba3cd2 100644 --- a/src/lru_cache.cpp +++ b/src/lru_cache.cpp @@ -1,10 +1,13 @@ #include "lru_cache.h" #include +#include #include #include +#include #include "map_memory.h" +#include "memory_fast.h" #include "point.h" template @@ -73,3 +76,4 @@ const std::list::Pair> &lru_cache::li template class lru_cache; template class lru_cache; template class lru_cache; +template class lru_cache>; diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 59b6c40443337..2000cacb3c269 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -2439,8 +2438,9 @@ void mapgen_function_json_base::setup_common() debugmsg( "null json source location path" ); return; } - std::ifstream ifs( *jsrcloc.path, std::ifstream::in | std::ifstream::binary ); - JsonIn jsin( ifs, jsrcloc ); + shared_ptr_fast stream = DynamicDataLoader::get_instance().get_cached_stream( + *jsrcloc.path ); + JsonIn jsin( *stream, jsrcloc ); JsonObject jo = jsin.get_object(); mapgen_defer::defer = false; if( !setup_common( jo ) ) {