From 40c2602b0ef06abb1f49a224bfaef2468ca8cb10 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Wed, 6 Feb 2019 13:29:56 +0100 Subject: [PATCH 1/7] Fix discrepancy with v06 --- src/ifcgeom/IfcGeomHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomHelpers.cpp b/src/ifcgeom/IfcGeomHelpers.cpp index 0e772b64c2..2517be14a0 100644 --- a/src/ifcgeom/IfcGeomHelpers.cpp +++ b/src/ifcgeom/IfcGeomHelpers.cpp @@ -169,7 +169,7 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcAxis2Placement3D* l, gp_Trsf& const bool hasRef = l->hasRefDirection(); if (hasAxis != hasRef) { - Logger::Warning("Axis and RefDirection should be specified together", l); + Logger::Warning("Axis and RefDirection should be specified together", l->entity); } if (hasAxis) { From 38f7ccdc201e3ed61a5085a9dd8eccbd375966a6 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Fri, 28 Sep 2018 15:00:48 +0300 Subject: [PATCH 2/7] Optimize IfcFile::entitiesByReference() by reserving capacity for IfcEntityList in advance. Around 18.4 % speed-up (avg. of first 2000 calls) when converting a somewhat large (176 MB) file to XML. --- src/ifcparse/IfcEntityList.h | 1 + src/ifcparse/IfcParse.cpp | 11 +++++------ src/ifcparse/IfcUtil.cpp | 1 + 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ifcparse/IfcEntityList.h b/src/ifcparse/IfcEntityList.h index 3953a29715..d8789fee4c 100644 --- a/src/ifcparse/IfcEntityList.h +++ b/src/ifcparse/IfcEntityList.h @@ -40,6 +40,7 @@ public: it end(); IfcUtil::IfcBaseClass* operator[] (int i); unsigned int size() const; + void reserve(unsigned capacity); bool contains(IfcUtil::IfcBaseClass*) const; template typename U::list::ptr as() { diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index 83b0740bdb..20d7cb53dd 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1793,17 +1793,16 @@ IfcEntityList::ptr IfcFile::entitiesByType(const std::string& t) { IfcEntityList::ptr IfcFile::entitiesByReference(int t) { entities_by_ref_t::const_iterator it = byref.find(t); - IfcEntityList::ptr return_value; + IfcEntityList::ptr ret; if (it != byref.end()) { + ret.reset(new IfcEntityList); + ret->reserve((unsigned)it->second.size()); const std::vector& ids = it->second; for (std::vector::const_iterator jt = ids.begin(); jt != ids.end(); ++jt) { - if (!return_value) { - return_value.reset(new IfcEntityList); - } - return_value->push(entityById(*jt)); + ret->push(entityById(*jt)); } } - return return_value; + return ret; } IfcUtil::IfcBaseClass* IfcFile::entityById(int id) { diff --git a/src/ifcparse/IfcUtil.cpp b/src/ifcparse/IfcUtil.cpp index 5c2546c5e7..fb4c48a815 100644 --- a/src/ifcparse/IfcUtil.cpp +++ b/src/ifcparse/IfcUtil.cpp @@ -48,6 +48,7 @@ void IfcEntityList::push(const IfcEntityList::ptr& l) { } } unsigned int IfcEntityList::size() const { return (unsigned int) ls.size(); } +void IfcEntityList::reserve(unsigned capacity) { ls.reserve((size_t)capacity); } IfcEntityList::it IfcEntityList::begin() { return ls.begin(); } IfcEntityList::it IfcEntityList::end() { return ls.end(); } IfcUtil::IfcBaseClass* IfcEntityList::operator[] (int i) { From 910cdb3754c00589de745944bd4e3f6ae161c83a Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Mon, 1 Oct 2018 13:19:31 +0300 Subject: [PATCH 3/7] IfcConvert: print durations of file parsing and XML conversion. --- src/ifcconvert/IfcConvert.cpp | 53 ++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index 4659875800..7847f139b8 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -116,6 +116,7 @@ bool rename_file(const std::string& old_filename, const std::string& new_filenam static std::stringstream log_stream; void write_log(bool); +std::string format_duration(time_t start, time_t end); /// @todo make the filters non-global IfcGeom::entity_filter entity_filter; // Entity filter is used always by default. @@ -473,11 +474,15 @@ int main(int argc, char** argv) int exit_code = EXIT_FAILURE; try { if (init_input_file(input_filename, ifc_file, no_progress || quiet, mmap)) { + time_t start, end; + time(&start); XmlSerializer s(output_temp_filename); s.setFile(&ifc_file); Logger::Status("Writing XML output..."); s.finalize(); - Logger::Status("Done!"); + time(&end); + Logger::Status("Done! Conversion took " + format_duration(start, end)); + rename_file(output_temp_filename, output_filename); exit_code = EXIT_SUCCESS; } @@ -760,28 +765,33 @@ int main(int argc, char** argv) time(&end); - if (!quiet) { - int seconds = (int)difftime(end, start); - std::stringstream msg; - int minutes = seconds / 60; - seconds = seconds % 60; - msg << "\nConversion took"; - if (minutes > 0) { - msg << " " << minutes << " minute"; - if (minutes > 1) { - msg << "s"; - } - } - msg << " " << seconds << " second"; - if (seconds > 1) { - msg << "s"; - } - Logger::Status(msg.str()); - } + if (!quiet) { + Logger::Status("\nConversion took " + format_duration(start, end)); + } return successful ? EXIT_SUCCESS : EXIT_FAILURE; } +std::string format_duration(time_t start, time_t end) +{ + int seconds = (int)difftime(end, start); + std::stringstream ss; + int minutes = seconds / 60; + seconds = seconds % 60; + if (minutes > 0) { + ss << minutes << " minute"; + if (minutes == 0 || minutes > 1) { + ss << "s"; + } + ss << " "; + } + ss << seconds << " second"; + if (seconds == 0 || seconds > 1) { + ss << "s"; + } + return ss.str(); +} + void write_log(bool header) { std::string log = log_stream.str(); if (!log.empty()) { @@ -794,9 +804,12 @@ void write_log(bool header) { bool init_input_file(const std::string &filename, IfcParse::IfcFile &ifc_file, bool no_progress, bool mmap) { + time_t start, end; + // Prevent IfcFile::Init() prints by setting output to null temporarily if (no_progress) { Logger::SetOutput(NULL, &log_stream); } + time(&start); #ifdef USE_MMAP if (!ifc_file.Init(filename, mmap)) { #else @@ -806,8 +819,10 @@ bool init_input_file(const std::string &filename, IfcParse::IfcFile &ifc_file, b Logger::Error("Unable to parse input file '" + filename + "'"); return false; } + time(&end); if (no_progress) { Logger::SetOutput(&std::cout, &log_stream); } + else { Logger::Status("Parsing input file took " + format_duration(start, end)); } return true; } From 9211f80625ce4882b7aa08ea16390cb46001d439 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Mon, 1 Oct 2018 15:11:34 +0300 Subject: [PATCH 4/7] IfcFile::entitiesByReference: cache loaded references. Profiling shows around 3x speed-up (avg. of first 5000 calls to this function). --- src/ifcparse/IfcFile.h | 2 ++ src/ifcparse/IfcParse.cpp | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/ifcparse/IfcFile.h b/src/ifcparse/IfcFile.h index daa31feb27..1e871412fe 100644 --- a/src/ifcparse/IfcFile.h +++ b/src/ifcparse/IfcFile.h @@ -39,6 +39,7 @@ public: typedef boost::unordered_map entity_by_id_t; typedef std::map entity_by_guid_t; typedef std::map > entities_by_ref_t; + typedef std::map ref_map_t; typedef entity_by_id_t::const_iterator const_iterator; class type_iterator : private entities_by_type_t::const_iterator { @@ -77,6 +78,7 @@ private: entities_by_type_t bytype; entities_by_type_t bytype_excl; entities_by_ref_t byref; + ref_map_t by_ref_cached_; entity_by_guid_t byguid; entity_entity_map_t entity_file_map; diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index 20d7cb53dd..cd904790af 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1795,12 +1795,19 @@ IfcEntityList::ptr IfcFile::entitiesByReference(int t) { entities_by_ref_t::const_iterator it = byref.find(t); IfcEntityList::ptr ret; if (it != byref.end()) { - ret.reset(new IfcEntityList); - ret->reserve((unsigned)it->second.size()); - const std::vector& ids = it->second; - for (std::vector::const_iterator jt = ids.begin(); jt != ids.end(); ++jt) { - ret->push(entityById(*jt)); - } + ref_map_t::const_iterator cached_it = by_ref_cached_.find(t); + if (cached_it != by_ref_cached_.end()) { + ret = cached_it->second; + } + else { + ret.reset(new IfcEntityList); + ret->reserve((unsigned)it->second.size()); + const std::vector& ids = it->second; + for (std::vector::const_iterator jt = ids.begin(); jt != ids.end(); ++jt) { + ret->push(entityById(*jt)); + } + by_ref_cached_[t] = ret; + } } return ret; } From 3bef32bb874b6f535164711bbefed85bdb8ac08f Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Mon, 1 Oct 2018 19:59:18 +0300 Subject: [PATCH 5/7] IfcGeom::Kernel::get_layers: remove what would appear to be unnecessary code (yields 0 layers in my tests). The LayerAssignments() calls can take up to 95 % of the function's execution time yielding no results. --- src/ifcgeom/IfcGeomFunctions.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 416a5d51d0..dc0a98e213 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -1807,20 +1807,6 @@ std::map IfcGeom::Kerne layers[(*jt)->Name()] = *jt; } } - - IfcRepresentationItem::list::ptr items = r->as(); - for (IfcRepresentationItem::list::it it = items->begin(); it != items->end(); ++it) { - IfcPresentationLayerAssignment::list::ptr a = (*it)-> - // LayerAssignments renamed from plural to singular, LayerAssignment, so work around that -#ifdef USE_IFC4 - LayerAssignment(); -#else - LayerAssignments(); -#endif - for (IfcPresentationLayerAssignment::list::it jt = a->begin(); jt != a->end(); ++jt) { - layers[(*jt)->Name()] = *jt; - } - } } return layers; } From 000d11daab57c2a264fbc3125a982ba5cbd4d71a Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Fri, 21 Dec 2018 12:06:35 +0200 Subject: [PATCH 6/7] IfcFile: possibility to mark an entity as modified so that potential cache is invalidated. --- src/ifcparse/IfcFile.h | 4 ++++ src/ifcparse/IfcParse.cpp | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/ifcparse/IfcFile.h b/src/ifcparse/IfcFile.h index 1e871412fe..66f5c86159 100644 --- a/src/ifcparse/IfcFile.h +++ b/src/ifcparse/IfcFile.h @@ -148,6 +148,10 @@ public: /// in the first function argument. IfcEntityList::ptr traverse(IfcUtil::IfcBaseClass* instance, int max_level=-1); + /// Marks entity as modified so that potential cache for it is invalidated. + /// @todo Currently the whole cache is invalidated. Implement more fine-grained invalidation. + void mark_entity_as_modified(int id); + #ifdef USE_MMAP bool Init(const std::string& fn, bool mmap=false); #else diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index cd904790af..9e2b3bb41d 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1213,7 +1213,9 @@ void IfcEntityInstanceData::setArgument(unsigned int i, Argument* a, IfcUtil::Ar if (this->file) { register_inverse_visitor visitor(*this->file, *this); apply_individual_instance_visitor(copy).apply(visitor); - } + + this->file->mark_entity_as_modified(id_); + } if (i < attributes_.size()) { attributes_[i] = copy; @@ -1429,6 +1431,11 @@ IfcEntityList::ptr IfcFile::traverse(IfcUtil::IfcBaseClass* instance, int max_le return IfcParse::traverse(instance, max_level); } +void IfcFile::mark_entity_as_modified(int /*id*/) +{ + by_ref_cached_.clear(); +} + void IfcFile::addEntities(IfcEntityList::ptr es) { for( IfcEntityList::it i = es->begin(); i != es->end(); ++ i ) { addEntity(*i); From 6bac067488c7d559371ab6d940b79f5e7c41f461 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Wed, 6 Feb 2019 13:33:38 +0100 Subject: [PATCH 7/7] Keep behaviour of returned null pointers for now --- src/ifcparse/IfcParse.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index 9e2b3bb41d..0e63c88360 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1807,11 +1807,13 @@ IfcEntityList::ptr IfcFile::entitiesByReference(int t) { ret = cached_it->second; } else { - ret.reset(new IfcEntityList); - ret->reserve((unsigned)it->second.size()); - const std::vector& ids = it->second; - for (std::vector::const_iterator jt = ids.begin(); jt != ids.end(); ++jt) { - ret->push(entityById(*jt)); + if (it->second.size()) { + ret.reset(new IfcEntityList); + ret->reserve((unsigned)it->second.size()); + const std::vector& ids = it->second; + for (std::vector::const_iterator jt = ids.begin(); jt != ids.end(); ++jt) { + ret->push(entityById(*jt)); + } } by_ref_cached_[t] = ret; }