optimized caching of items in mmcif::Atom

include/cif++/Cif++.hpp

@@ -36,6 +36,7 @@
 #include <regex>
 #include <regex>
 #include <set>
 #include <set>
 #include <sstream>
 #include <sstream>
+#include <shared_mutex>
 
 
 #include "cif++/CifUtils.hpp"
 #include "cif++/CifUtils.hpp"
 
 
@@ -142,13 +143,13 @@ class Item
 	Item() {}
 	Item() {}
 
 
 	template <typename T, std::enable_if_t<std::is_arithmetic_v<T>, int> = 0>
 	template <typename T, std::enable_if_t<std::is_arithmetic_v<T>, int> = 0>
-	Item(const std::string &name, const T &value)
+	Item(const std::string_view name, const T &value)
 		: mName(name)
 		: mName(name)
 		, mValue(std::to_string(value))
 		, mValue(std::to_string(value))
 	{
 	{
 	}
 	}
 
 
-	Item(const std::string &name, const std::string &value)
+	Item(const std::string_view name, const std::string_view value)
 		: mName(name)
 		: mName(name)
 		, mValue(value)
 		, mValue(value)
 	{
 	{
@@ -221,7 +222,7 @@ class Datablock
 	using iterator = CategoryList::iterator;
 	using iterator = CategoryList::iterator;
 	using const_iterator = CategoryList::const_iterator;
 	using const_iterator = CategoryList::const_iterator;
 
 
-	Datablock(const std::string &name);
+	Datablock(const std::string_view name);
 	~Datablock();
 	~Datablock();
 
 
 	Datablock(const Datablock &) = delete;
 	Datablock(const Datablock &) = delete;
@@ -230,8 +231,6 @@ class Datablock
 	std::string getName() const { return mName; }
 	std::string getName() const { return mName; }
 	void setName(const std::string &n) { mName = n; }
 	void setName(const std::string &n) { mName = n; }
 
 
-	std::string firstItem(const std::string &tag) const;
-
 	iterator begin() { return mCategories.begin(); }
 	iterator begin() { return mCategories.begin(); }
 	iterator end() { return mCategories.end(); }
 	iterator end() { return mCategories.end(); }
 
 
@@ -256,7 +255,7 @@ class Datablock
 	void write(std::ostream &os);
 	void write(std::ostream &os);
 
 
 	// convenience function, add a line to the software category
 	// convenience function, add a line to the software category
-	void add_software(const std::string &name, const std::string &classification,
+	void add_software(const std::string_view name, const std::string &classification,
 		const std::string &versionNr, const std::string &versionDate);
 		const std::string &versionNr, const std::string &versionDate);
 
 
 	friend bool operator==(const Datablock &lhs, const Datablock &rhs);
 	friend bool operator==(const Datablock &lhs, const Datablock &rhs);
@@ -264,7 +263,8 @@ class Datablock
 	friend std::ostream& operator<<(std::ostream &os, const Datablock &data);
 	friend std::ostream& operator<<(std::ostream &os, const Datablock &data);
 
 
   private:
   private:
-	std::list<Category> mCategories;
+	CategoryList mCategories;		// LRU
+	mutable std::shared_mutex mLock;
 	std::string mName;
 	std::string mName;
 	Validator *mValidator;
 	Validator *mValidator;
 	Datablock *mNext;
 	Datablock *mNext;
@@ -1816,12 +1816,12 @@ class Category
 	friend class Row;
 	friend class Row;
 	friend class detail::ItemReference;
 	friend class detail::ItemReference;
 
 
-	Category(Datablock &db, const std::string &name, Validator *Validator);
+	Category(Datablock &db, const std::string_view name, Validator *Validator);
 	Category(const Category &) = delete;
 	Category(const Category &) = delete;
 	Category &operator=(const Category &) = delete;
 	Category &operator=(const Category &) = delete;
 	~Category();
 	~Category();
 
 
-	const std::string name() const { return mName; }
+	const std::string &name() const { return mName; }
 
 
 	using iterator = iterator_impl<Row>;
 	using iterator = iterator_impl<Row>;
 	using const_iterator = iterator_impl<const Row>;
 	using const_iterator = iterator_impl<const Row>;

include/cif++/Structure.hpp

@@ -109,12 +109,12 @@ class Atom
 	float occupancy() const;
 	float occupancy() const;
 
 
 	template <typename T>
 	template <typename T>
-	T property(const std::string &name) const;
+	T property(const std::string_view name) const;
 
 
-	void property(const std::string &name, const std::string &value);
+	void property(const std::string_view name, const std::string &value);
 
 
 	template <typename T, std::enable_if_t<std::is_arithmetic_v<T>, int> = 0>
 	template <typename T, std::enable_if_t<std::is_arithmetic_v<T>, int> = 0>
-	void property(const std::string &name, const T &value)
+	void property(const std::string_view name, const T &value)
 	{
 	{
 		property(name, std::to_string(value));
 		property(name, std::to_string(value));
 	}
 	}
@@ -404,7 +404,7 @@ class File : public std::enable_shared_from_this<File>
 	File(const File &) = delete;
 	File(const File &) = delete;
 	File &operator=(const File &) = delete;
 	File &operator=(const File &) = delete;
 
 
-	cif::Datablock& createDatablock(const std::string &name);
+	cif::Datablock& createDatablock(const std::string_view name);
 
 
 	void load(const std::filesystem::path &path);
 	void load(const std::filesystem::path &path);
 	void save(const std::filesystem::path &path);
 	void save(const std::filesystem::path &path);

src/Cif++.cpp

@@ -33,6 +33,7 @@
 #include <stack>
 #include <stack>
 #include <tuple>
 #include <tuple>
 #include <unordered_map>
 #include <unordered_map>
+#include <shared_mutex>
 
 
 #include <filesystem>
 #include <filesystem>
 
 
@@ -351,7 +352,7 @@ namespace detail
 // --------------------------------------------------------------------
 // --------------------------------------------------------------------
 // Datablock implementation
 // Datablock implementation
 
 
-Datablock::Datablock(const std::string &name)
+Datablock::Datablock(const std::string_view name)
 	: mName(name)
 	: mName(name)
 	, mValidator(nullptr)
 	, mValidator(nullptr)
 	, mNext(nullptr)
 	, mNext(nullptr)
@@ -363,43 +364,57 @@ Datablock::~Datablock()
 	delete mNext;
 	delete mNext;
 }
 }
 
 
-std::string Datablock::firstItem(const std::string &tag) const
+auto Datablock::emplace(std::string_view name) -> std::tuple<iterator, bool>
 {
 {
-	std::string result;
+	// LRU code
 
 
-	std::string catName, itemName;
-	std::tie(catName, itemName) = splitTagName(tag);
+	std::shared_lock lock(mLock);
 
 
-	for (auto &cat : mCategories)
+	bool isNew = true;
+
+	auto i = begin();
+	while (i != end())
 	{
 	{
-		if (iequals(cat.name(), catName))
+		if (iequals(name, i->name()))
 		{
 		{
-			for (auto row : cat)
+			isNew = false;
+
+			if (i != begin())
 			{
 			{
-				result = row[itemName].as<std::string>();
-				break;
+				auto n = std::next(i);
+				mCategories.splice(begin(), mCategories, i, n);
 			}
 			}
 
 
 			break;
 			break;
 		}
 		}
+
+		++i;
 	}
 	}
 
 
-	return result;
-}
+	if (isNew)
+		mCategories.emplace(begin(), *this, std::string(name), mValidator);
 
 
-auto Datablock::emplace(std::string_view name) -> std::tuple<iterator, bool>
-{
-	bool isNew = false;
-	iterator i = find_if(begin(), end(), [name](const Category &cat) -> bool
-		{ return iequals(cat.name(), name); });
+	return std::make_tuple(begin(), isNew);
 
 
-	if (i == end())
-	{
-		isNew = true;
-		i = mCategories.emplace(end(), *this, std::string(name), mValidator);
-	}
 
 
-	return std::make_tuple(i, isNew);
+	// bool isNew = false;
+
+	// iterator i = find_if(begin(), end(), [name](const Category &cat) -> bool
+	// 	{ return iequals(cat.name(), name); });
+
+	// if (i == end())
+	// {
+	// 	isNew = true;
+	// 	i = mCategories.emplace(begin(), *this, std::string(name), mValidator);
+	// }
+	// else if (i != begin())
+	// {
+	// 	auto n = std::next(i);
+	// 	mCategories.splice(begin(), mCategories, i, n);
+	// 	i = mCategories.begin();
+	// }
+
+	// return std::make_tuple(i, isNew);
 }
 }
 
 
 Category &Datablock::operator[](std::string_view name)
 Category &Datablock::operator[](std::string_view name)
@@ -411,14 +426,13 @@ Category &Datablock::operator[](std::string_view name)
 
 
 Category *Datablock::get(std::string_view name)
 Category *Datablock::get(std::string_view name)
 {
 {
-	auto i = find_if(begin(), end(), [name](const Category &cat) -> bool
-		{ return iequals(cat.name(), name); });
-
-	return i == end() ? nullptr : &*i;
+	return &operator[](name);
 }
 }
 
 
 const Category *Datablock::get(std::string_view name) const
 const Category *Datablock::get(std::string_view name) const
 {
 {
+	std::shared_lock lock(mLock);
+
 	auto i = find_if(begin(), end(), [name](const Category &cat) -> bool
 	auto i = find_if(begin(), end(), [name](const Category &cat) -> bool
 		{ return iequals(cat.name(), name); });
 		{ return iequals(cat.name(), name); });
 
 
@@ -427,6 +441,8 @@ const Category *Datablock::get(std::string_view name) const
 
 
 bool Datablock::isValid()
 bool Datablock::isValid()
 {
 {
+	std::shared_lock lock(mLock);
+
 	if (mValidator == nullptr)
 	if (mValidator == nullptr)
 		throw std::runtime_error("Validator not specified");
 		throw std::runtime_error("Validator not specified");
 
 
@@ -438,20 +454,26 @@ bool Datablock::isValid()
 
 
 void Datablock::validateLinks() const
 void Datablock::validateLinks() const
 {
 {
+	std::shared_lock lock(mLock);
+
 	for (auto &cat : *this)
 	for (auto &cat : *this)
 		cat.validateLinks();
 		cat.validateLinks();
 }
 }
 
 
 void Datablock::setValidator(Validator *v)
 void Datablock::setValidator(Validator *v)
 {
 {
+	std::shared_lock lock(mLock);
+
 	mValidator = v;
 	mValidator = v;
 
 
 	for (auto &cat : *this)
 	for (auto &cat : *this)
 		cat.setValidator(v);
 		cat.setValidator(v);
 }
 }
 
 
-void Datablock::add_software(const std::string &name, const std::string &classification, const std::string &versionNr, const std::string &versionDate)
+void Datablock::add_software(const std::string_view name, const std::string &classification, const std::string &versionNr, const std::string &versionDate)
 {
 {
+	std::shared_lock lock(mLock);
+
 	Category &cat = operator[]("software");
 	Category &cat = operator[]("software");
 	auto ordNr = cat.size() + 1;
 	auto ordNr = cat.size() + 1;
 	// TODO: should we check this ordinal number???
 	// TODO: should we check this ordinal number???
@@ -465,12 +487,16 @@ void Datablock::add_software(const std::string &name, const std::string &classif
 
 
 void Datablock::getTagOrder(std::vector<std::string> &tags) const
 void Datablock::getTagOrder(std::vector<std::string> &tags) const
 {
 {
+	std::shared_lock lock(mLock);
+
 	for (auto &cat : *this)
 	for (auto &cat : *this)
 		cat.getTagOrder(tags);
 		cat.getTagOrder(tags);
 }
 }
 
 
 void Datablock::write(std::ostream &os)
 void Datablock::write(std::ostream &os)
 {
 {
+	std::shared_lock lock(mLock);
+
 	os << "data_" << mName << std::endl
 	os << "data_" << mName << std::endl
 	   << "# " << std::endl;
 	   << "# " << std::endl;
 
 
@@ -505,6 +531,8 @@ void Datablock::write(std::ostream &os)
 
 
 void Datablock::write(std::ostream &os, const std::vector<std::string> &order)
 void Datablock::write(std::ostream &os, const std::vector<std::string> &order)
 {
 {
+	std::shared_lock lock(mLock);
+
 	os << "data_" << mName << std::endl
 	os << "data_" << mName << std::endl
 	   << "# " << std::endl;
 	   << "# " << std::endl;
 
 
@@ -580,6 +608,9 @@ void Datablock::write(std::ostream &os, const std::vector<std::string> &order)
 
 
 bool operator==(const cif::Datablock &dbA, const cif::Datablock &dbB)
 bool operator==(const cif::Datablock &dbA, const cif::Datablock &dbB)
 {
 {
+	std::shared_lock lockA(dbA.mLock);
+	std::shared_lock lockB(dbB.mLock);
+
 	std::vector<std::string> catA, catB;
 	std::vector<std::string> catA, catB;
 
 
 	for (auto &cat : dbA)
 	for (auto &cat : dbA)
@@ -1311,7 +1342,7 @@ RowSet &RowSet::orderBy(std::initializer_list<std::string> items)
 
 
 // --------------------------------------------------------------------
 // --------------------------------------------------------------------
 
 
-Category::Category(Datablock &db, const std::string &name, Validator *Validator)
+Category::Category(Datablock &db, const std::string_view name, Validator *Validator)
 	: mDb(db)
 	: mDb(db)
 	, mName(name)
 	, mName(name)
 	, mValidator(Validator)
 	, mValidator(Validator)

src/CifParser.cpp

@@ -976,7 +976,8 @@ void DictParser::parseSaveFrame()
 	
 	
 	if (isCategorySaveFrame)
 	if (isCategorySaveFrame)
 	{
 	{
-		std::string category = dict.firstItem("_category.id");
+		std::string category;
+		cif::tie(category) = dict["category"].front().get("id");
 
 
 		std::vector<std::string> keys;
 		std::vector<std::string> keys;
 		for (auto k: dict["category_key"])
 		for (auto k: dict["category_key"])
@@ -991,7 +992,8 @@ void DictParser::parseSaveFrame()
 	else
 	else
 	{
 	{
 		// if the type code is missing, this must be a pointer, just skip it
 		// if the type code is missing, this must be a pointer, just skip it
-		std::string typeCode = dict.firstItem("_item_type.code");
+		std::string typeCode;
+		cif::tie(typeCode) = dict["item_type"].front().get("code");
 
 
 		const ValidateType* tv = nullptr;
 		const ValidateType* tv = nullptr;
 		if (not (typeCode.empty() or typeCode == "?"))
 		if (not (typeCode.empty() or typeCode == "?"))
@@ -1001,7 +1003,8 @@ void DictParser::parseSaveFrame()
 		for (auto e: dict["item_enumeration"])
 		for (auto e: dict["item_enumeration"])
 			ess.insert(e["value"].as<std::string>());
 			ess.insert(e["value"].as<std::string>());
 		
 		
-		std::string defaultValue = dict.firstItem("_item_default.value");
+		std::string defaultValue;
+		cif::tie(defaultValue) = dict["item_default"].front().get("value");
 		bool defaultIsNull = false;
 		bool defaultIsNull = false;
 		if (defaultValue.empty())
 		if (defaultValue.empty())
 		{
 		{

src/Point.cpp

@@ -102,14 +102,14 @@ Point CenterPoints(std::vector<Point>& Points)
 	return t;
 	return t;
 }
 }
 
 
-Point Centroid(std::vector<Point>& Points)
+Point Centroid(const std::vector<Point>& pts)
 {
 {
 	Point result;
 	Point result;
 	
 	
-	for (Point& pt : Points)
+	for (auto &pt : pts)
 		result += pt;
 		result += pt;
 	
 	
-	result /= static_cast<float>(Points.size());
+	result /= static_cast<float>(pts.size());
 	
 	
 	return result;
 	return result;
 }
 }

src/Structure.cpp

@@ -216,9 +216,9 @@ struct AtomImpl
 		, mLocation(i.mLocation)
 		, mLocation(i.mLocation)
 		, mRefcount(1)
 		, mRefcount(1)
 		, mRow(i.mRow)
 		, mRow(i.mRow)
+		, mCachedRefs(i.mCachedRefs)
 		, mCompound(i.mCompound)
 		, mCompound(i.mCompound)
 		, mRadius(i.mRadius)
 		, mRadius(i.mRadius)
-		, mCachedProperties(i.mCachedProperties)
 		, mSymmetryCopy(i.mSymmetryCopy)
 		, mSymmetryCopy(i.mSymmetryCopy)
 		, mClone(true)
 		, mClone(true)
 	// , mRTop(i.mRTop), mD(i.mD)
 	// , mRTop(i.mRTop), mD(i.mD)
@@ -270,9 +270,9 @@ struct AtomImpl
 		, mLocation(loc)
 		, mLocation(loc)
 		, mRefcount(1)
 		, mRefcount(1)
 		, mRow(impl.mRow)
 		, mRow(impl.mRow)
+		, mCachedRefs(impl.mCachedRefs)
 		, mCompound(impl.mCompound)
 		, mCompound(impl.mCompound)
 		, mRadius(impl.mRadius)
 		, mRadius(impl.mRadius)
-		, mCachedProperties(impl.mCachedProperties)
 		, mSymmetryCopy(true)
 		, mSymmetryCopy(true)
 		, mSymmetryOperator(sym_op)
 		, mSymmetryOperator(sym_op)
 	{
 	{
@@ -338,9 +338,9 @@ struct AtomImpl
 
 
 		if (not mClone)
 		if (not mClone)
 		{
 		{
-			mRow["Cartn_x"] = p.getX();
-			mRow["Cartn_y"] = p.getY();
-			mRow["Cartn_z"] = p.getZ();
+			property("Cartn_x", std::to_string(p.getX()));
+			property("Cartn_y", std::to_string(p.getY()));
+			property("Cartn_z", std::to_string(p.getZ()));
 		}
 		}
 
 
 		//		boost::format kPosFmt("%.3f");
 		//		boost::format kPosFmt("%.3f");
@@ -382,26 +382,31 @@ struct AtomImpl
 		return mRadius;
 		return mRadius;
 	}
 	}
 
 
-	const std::string &property(const std::string &name) const
+	const std::string property(const std::string_view name) const
 	{
 	{
-		static std::string kEmptyString;
-
-		auto i = mCachedProperties.find(name);
-		if (i == mCachedProperties.end())
+		for (auto &&[tag, ref] : mCachedRefs)
 		{
 		{
-			auto v = mRow[name];
-			if (v.empty())
-				return kEmptyString;
-
-			return mCachedProperties[name] = v.as<std::string>();
+			if (tag == name)
+				return ref.as<std::string>();
 		}
 		}
-		else
-			return i->second;
+
+		mCachedRefs.emplace_back(name, mRow[name]);
+		return std::get<1>(mCachedRefs.back()).as<std::string>();
 	}
 	}
 
 
-	void property(const std::string &name, const std::string &value)
+	void property(const std::string_view name, const std::string &value)
 	{
 	{
-		mRow[name] = value;
+		for (auto &&[tag, ref] : mCachedRefs)
+		{
+			if (tag != name)
+				continue;
+			
+			ref = value;
+			return;
+		}
+
+		mCachedRefs.emplace_back(name, mRow[name]);
+		std::get<1>(mCachedRefs.back()) = value;
 	}
 	}
 
 
 	int compare(const AtomImpl &b) const
 	int compare(const AtomImpl &b) const
@@ -432,9 +437,11 @@ struct AtomImpl
 	Point mLocation;
 	Point mLocation;
 	int mRefcount;
 	int mRefcount;
 	cif::Row mRow;
 	cif::Row mRow;
+
+	mutable std::vector<std::tuple<std::string,cif::detail::ItemReference>> mCachedRefs;
+
 	mutable const Compound *mCompound = nullptr;
 	mutable const Compound *mCompound = nullptr;
 	float mRadius = std::nanf("4");
 	float mRadius = std::nanf("4");
-	mutable std::map<std::string, std::string> mCachedProperties;
 
 
 	bool mSymmetryCopy = false;
 	bool mSymmetryCopy = false;
 	bool mClone = false;
 	bool mClone = false;
@@ -533,25 +540,25 @@ const cif::Row Atom::getRowAniso() const
 }
 }
 
 
 template <>
 template <>
-std::string Atom::property<std::string>(const std::string &name) const
+std::string Atom::property<std::string>(const std::string_view name) const
 {
 {
 	return impl()->property(name);
 	return impl()->property(name);
 }
 }
 
 
 template <>
 template <>
-int Atom::property<int>(const std::string &name) const
+int Atom::property<int>(const std::string_view name) const
 {
 {
 	auto v = impl()->property(name);
 	auto v = impl()->property(name);
 	return v.empty() ? 0 : stoi(v);
 	return v.empty() ? 0 : stoi(v);
 }
 }
 
 
 template <>
 template <>
-float Atom::property<float>(const std::string &name) const
+float Atom::property<float>(const std::string_view name) const
 {
 {
 	return stof(impl()->property(name));
 	return stof(impl()->property(name));
 }
 }
 
 
-void Atom::property(const std::string &name, const std::string &value)
+void Atom::property(const std::string_view name, const std::string &value)
 {
 {
 	impl()->property(name, value);
 	impl()->property(name, value);
 }
 }
@@ -1736,7 +1743,7 @@ File::~File()
 	delete mImpl;
 	delete mImpl;
 }
 }
 
 
-cif::Datablock& File::createDatablock(const std::string &name)
+cif::Datablock& File::createDatablock(const std::string_view name)
 {
 {
 	auto db = new cif::Datablock(name);
 	auto db = new cif::Datablock(name);
 
 
@@ -1807,9 +1814,9 @@ Structure::Structure(File &f, size_t modelNr, StructureOpenOptions options)
 	}
 	}
 
 
 	if (mAtoms.empty())
 	if (mAtoms.empty())
-		throw std::runtime_error("No atoms loaded, refuse to continue");
-
-	loadData();
+		std::cerr << "Warning: no atoms loaded" << std::endl;
+	else
+		loadData();
 }
 }
 
 
 void Structure::loadAtomsForModel(StructureOpenOptions options)
 void Structure::loadAtomsForModel(StructureOpenOptions options)