Eliminate redundant associative container lookups across the compiler

CODING_STYLE.md

@@ -229,7 +229,47 @@ Example:
 
 See [this issue](https://stackoverflow.com/questions/614302/c-header-order/614333#614333 "C header order") for the reason: this makes it easier to find missing includes in header files.
 
-## 13. Recommended reading
+## 13. Associative Containers
+
+1. Prefer `std::unordered_map` / `std::unordered_set` over `std::map` / `std::set` when iteration order does not matter (i.e. the container is used only for lookup, membership testing, or deduplication).
+2. Avoid multiple sequential lookups on the same key. Use a single `find` and reuse the iterator:
+
+Yes:
+```cpp
+if (auto it = m_map.find(key); it != m_map.end())
+	use(it->second);
+```
+
+No:
+```cpp
+if (m_map.count(key))
+	use(m_map.at(key));
+```
+
+3. For "insert if absent" patterns, prefer `insert().second` or `try_emplace` over `count` + `insert`/`operator[]`:
+
+Yes:
+```cpp
+if (auto [it, inserted] = m_set.insert(value); inserted)
+	onNewElement();
+
+auto [it, inserted] = m_map.try_emplace(key);
+if (inserted)
+	it->second = computeValue();
+```
+
+No:
+```cpp
+if (!m_set.count(value))
+{
+	m_set.insert(value);
+	onNewElement();
+}
+```
+
+4. Use `std::erase_if` (C++20) instead of manual erase-while-iterate loops on associative containers.
+
+## 14. Recommended reading
 
 - Herb Sutter and Bjarne Stroustrup:
   - [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md)

libevmasm/Assembly.cpp

@@ -741,15 +741,18 @@ std::shared_ptr<std::string const> Assembly::sharedSourceName(std::string const&
 AssemblyItem Assembly::namedTag(std::string const& _name, size_t _params, size_t _returns, std::optional<uint64_t> _sourceID)
 {
 	assertThrow(!_name.empty(), AssemblyException, "Empty named tag.");
-	if (m_namedTags.count(_name))
+	if (auto it = m_namedTags.find(_name); it != m_namedTags.end())
 	{
-		assertThrow(m_namedTags.at(_name).params == _params, AssemblyException, "");
-		assertThrow(m_namedTags.at(_name).returns == _returns, AssemblyException, "");
-		assertThrow(m_namedTags.at(_name).sourceID == _sourceID, AssemblyException, "");
+		assertThrow(it->second.params == _params, AssemblyException, "");
+		assertThrow(it->second.returns == _returns, AssemblyException, "");
+		assertThrow(it->second.sourceID == _sourceID, AssemblyException, "");
+		return AssemblyItem{Tag, it->second.id};
 	}
-	else
-		m_namedTags[_name] = {static_cast<size_t>(newTag().data()), _sourceID, _params, _returns};
-	return AssemblyItem{Tag, m_namedTags.at(_name).id};
+	auto [it, _] = m_namedTags.emplace(
+		_name,
+		NamedTagInfo{static_cast<size_t>(newTag().data()), _sourceID, _params, _returns}
+	);
+	return AssemblyItem{Tag, it->second.id};
 }
 
 AssemblyItem Assembly::newFunctionCall(uint16_t _functionID) const

libevmasm/Assembly.h

@@ -39,6 +39,7 @@
 #include <sstream>
 #include <memory>
 #include <map>
+#include <unordered_map>
 #include <utility>
 
 namespace solidity::evmasm
@@ -303,7 +304,7 @@ class Assembly
 		size_t returns;
 	};
 
-	std::map<std::string, NamedTagInfo> m_namedTags;
+	std::unordered_map<std::string, NamedTagInfo> m_namedTags;
 	std::map<util::h256, bytes> m_data;
 	/// Data that is appended to the very end of the contract.
 	bytes m_auxiliaryData;

libevmasm/CommonSubexpressionEliminator.cpp

@@ -179,7 +179,7 @@ AssemblyItems CSECodeGenerator::generateCode(
 	// generate the target stack elements
 	for (auto const& targetItem: m_targetStack)
 	{
-		if (m_stack.count(targetItem.first) && m_stack.at(targetItem.first) == targetItem.second)
+		if (auto it = m_stack.find(targetItem.first); it != m_stack.end() && it->second == targetItem.second)
 			continue; // already there
 		generateClassElement(targetItem.second);
 		assertThrow(!m_classPositions[targetItem.second].empty(), OptimizerException, "");
@@ -427,12 +427,13 @@ void CSECodeGenerator::generateClassElement(Id _c, bool _allowSequenced)
 
 int CSECodeGenerator::classElementPosition(Id _id) const
 {
+	auto it = m_classPositions.find(_id);
 	assertThrow(
-		m_classPositions.count(_id) && !m_classPositions.at(_id).empty(),
+		it != m_classPositions.end() && !it->second.empty(),
 		OptimizerException,
 		"Element requested but is not present."
 	);
-	return *max_element(m_classPositions.at(_id).begin(), m_classPositions.at(_id).end());
+	return *max_element(it->second.begin(), it->second.end());
 }
 
 bool CSECodeGenerator::canBeRemoved(Id _element, Id _result, int _fromPosition)
@@ -443,8 +444,11 @@ bool CSECodeGenerator::canBeRemoved(Id _element, Id _result, int _fromPosition)
 
 	bool haveCopy = m_classPositions.at(_element).size() > 1;
 	if (m_finalClasses.count(_element))
+	{
 		// It is part of the target stack. It can be removed if it is a copy that is not in the target position.
-		return haveCopy && (!m_targetStack.count(_fromPosition) || m_targetStack[_fromPosition] != _element);
+		auto tsIt = m_targetStack.find(_fromPosition);
+		return haveCopy && (tsIt == m_targetStack.end() || tsIt->second != _element);
+	}
 	else if (!haveCopy)
 	{
 		// Can be removed unless it is needed by a class that has not been computed yet.

libevmasm/ControlFlowGraph.cpp

@@ -122,13 +122,16 @@ void ControlFlowGraph::resolveNextLinks()
 		{
 		case BasicBlock::EndType::JUMPI:
 		case BasicBlock::EndType::HANDOVER:
+		{
+			auto it = blockByBeginPos.find(block.end);
 			assertThrow(
-				blockByBeginPos.count(block.end),
+				it != blockByBeginPos.end(),
 				OptimizerException,
 				"Successor block not found."
 			);
-			block.next = blockByBeginPos.at(block.end);
+			block.next = it->second;
 			break;
+		}
 		default:
 			break;
 		}
@@ -193,15 +196,22 @@ void ControlFlowGraph::setPrevLinks()
 		if (push.type() != PushTag)
 			continue;
 		BlockId nextId(push.data());
-		if (m_blocks.count(nextId) && m_blocks.at(nextId).prev)
+		auto nextIt = m_blocks.find(nextId);
+		if (nextIt != m_blocks.end() && nextIt->second.prev)
 			continue;
 		bool hasLoop = false;
-		for (BlockId id = nextId; id && m_blocks.count(id) && !hasLoop; id = m_blocks.at(id).next)
+		for (BlockId id = nextId; id && !hasLoop;)
+		{
+			auto loopIt = m_blocks.find(id);
+			if (loopIt == m_blocks.end())
+				break;
 			hasLoop = (id == blockId);
-		if (hasLoop || !m_blocks.count(nextId))
+			id = loopIt->second.next;
+		}
+		if (hasLoop || nextIt == m_blocks.end())
 			continue;
 
-		m_blocks[nextId].prev = blockId;
+		nextIt->second.prev = blockId;
 		block.next = nextId;
 		block.end -= 2;
 		assertThrow(
@@ -244,9 +254,10 @@ void ControlFlowGraph::gatherKnowledge()
 		workQueue.pop_back();
 		//@todo we might have to do something like incrementing the sequence number for each JUMPDEST
 		assertThrow(!!item.blockId, OptimizerException, "");
-		if (!m_blocks.count(item.blockId))
+		auto blockIt = m_blocks.find(item.blockId);
+		if (blockIt == m_blocks.end())
 			continue; // too bad, we do not know the tag, probably an invalid jump
-		BasicBlock& block = m_blocks.at(item.blockId);
+		BasicBlock& block = blockIt->second;
 		KnownStatePointer state = item.state;
 		if (block.startState)
 		{

libevmasm/KnownState.cpp

@@ -215,20 +215,19 @@ KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool
 /// _this which is not in or not equal to the value in _other.
 template <class Mapping> void intersect(Mapping& _this, Mapping const& _other)
 {
-	for (auto it = _this.begin(); it != _this.end();)
-		if (_other.count(it->first) && _other.at(it->first) == it->second)
-			++it;
-		else
-			it = _this.erase(it);
+	std::erase_if(_this, [&](auto const& elem) {
+		auto it = _other.find(elem.first);
+		return it == _other.end() || it->second != elem.second;
+	});
 }
 
 void KnownState::reduceToCommonKnowledge(KnownState const& _other, bool _combineSequenceNumbers)
 {
 	int stackDiff = m_stackHeight - _other.m_stackHeight;
 	for (auto it = m_stackElements.begin(); it != m_stackElements.end();)
-		if (_other.m_stackElements.count(it->first - stackDiff))
+		if (auto otherIt = _other.m_stackElements.find(it->first - stackDiff); otherIt != _other.m_stackElements.end())
 		{
-			Id other = _other.m_stackElements.at(it->first - stackDiff);
+			Id other = otherIt->second;
 			if (it->second == other)
 				++it;
 			else
@@ -279,9 +278,8 @@ bool KnownState::operator==(KnownState const& _other) const
 
 ExpressionClasses::Id KnownState::stackElement(int _stackHeight, langutil::DebugData::ConstPtr _debugData)
 {
-	if (m_stackElements.count(_stackHeight))
-		return m_stackElements.at(_stackHeight);
-	// Stack element not found (not assigned yet), create new unknown equivalence class.
+	if (auto it = m_stackElements.find(_stackHeight); it != m_stackElements.end())
+		return it->second;
 	return m_stackElements[_stackHeight] =
 			m_expressionClasses->find(AssemblyItem(UndefinedItem, _stackHeight, std::move(_debugData)));
 }
@@ -325,8 +323,7 @@ KnownState::StoreOperation KnownState::storeInStorage(
 	langutil::DebugData::ConstPtr _debugData
 )
 {
-	if (m_storageContent.count(_slot) && m_storageContent[_slot] == _value)
-		// do not execute the storage if we know that the value is already there
+	if (auto it = m_storageContent.find(_slot); it != m_storageContent.end() && it->second == _value)
 		return StoreOperation();
 	m_sequenceNumber++;
 	decltype(m_storageContent) storageContents;
@@ -350,17 +347,16 @@ KnownState::StoreOperation KnownState::storeInStorage(
 
 ExpressionClasses::Id KnownState::loadFromStorage(Id _slot, langutil::DebugData::ConstPtr _debugData)
 {
-	if (m_storageContent.count(_slot))
-		return m_storageContent.at(_slot);
+	if (auto it = m_storageContent.find(_slot); it != m_storageContent.end())
+		return it->second;
 
 	AssemblyItem item(Instruction::SLOAD, std::move(_debugData));
 	return m_storageContent[_slot] = m_expressionClasses->find(item, {_slot}, true, m_sequenceNumber);
 }
 
 KnownState::StoreOperation KnownState::storeInMemory(Id _slot, Id _value, langutil::DebugData::ConstPtr _debugData)
 {
-	if (m_memoryContent.count(_slot) && m_memoryContent[_slot] == _value)
-		// do not execute the store if we know that the value is already there
+	if (auto it = m_memoryContent.find(_slot); it != m_memoryContent.end() && it->second == _value)
 		return StoreOperation();
 	m_sequenceNumber++;
 	decltype(m_memoryContent) memoryContents;
@@ -381,8 +377,8 @@ KnownState::StoreOperation KnownState::storeInMemory(Id _slot, Id _value, langut
 
 ExpressionClasses::Id KnownState::loadFromMemory(Id _slot, langutil::DebugData::ConstPtr _debugData)
 {
-	if (m_memoryContent.count(_slot))
-		return m_memoryContent.at(_slot);
+	if (auto it = m_memoryContent.find(_slot); it != m_memoryContent.end())
+		return it->second;
 
 	AssemblyItem item(Instruction::MLOAD, std::move(_debugData));
 	return m_memoryContent[_slot] = m_expressionClasses->find(item, {_slot}, true, m_sequenceNumber);
@@ -410,8 +406,8 @@ KnownState::Id KnownState::applyKeccak256(
 		);
 		arguments.push_back(loadFromMemory(slot, _debugData));
 	}
-	if (m_knownKeccak256Hashes.count({arguments, length}))
-		return m_knownKeccak256Hashes.at({arguments, length});
+	if (auto it = m_knownKeccak256Hashes.find({arguments, length}); it != m_knownKeccak256Hashes.end())
+		return it->second;
 	Id v;
 	// If all arguments are known constants, compute the Keccak-256 here
 	if (all_of(arguments.begin(), arguments.end(), [this](Id _a) { return !!m_expressionClasses->knownConstant(_a); }))
@@ -429,8 +425,8 @@ KnownState::Id KnownState::applyKeccak256(
 
 std::set<u256> KnownState::tagsInExpression(KnownState::Id _expressionId)
 {
-	if (m_tagUnions.left.count(_expressionId))
-		return m_tagUnions.left.at(_expressionId);
+	if (auto it = m_tagUnions.left.find(_expressionId); it != m_tagUnions.left.end())
+		return it->second;
 	// Might be a tag, then return the set of itself.
 	ExpressionClasses::Expression expr = m_expressionClasses->representative(_expressionId);
 	if (expr.item && expr.item->type() == PushTag)
@@ -441,8 +437,8 @@ std::set<u256> KnownState::tagsInExpression(KnownState::Id _expressionId)
 
 KnownState::Id KnownState::tagUnion(std::set<u256> _tags)
 {
-	if (m_tagUnions.right.count(_tags))
-		return m_tagUnions.right.at(_tags);
+	if (auto it = m_tagUnions.right.find(_tags); it != m_tagUnions.right.end())
+		return it->second;
 	else
 	{
 		Id id = m_expressionClasses->newClass(langutil::DebugData::create());

libevmasm/PathGasMeter.cpp

@@ -53,12 +53,13 @@ GasMeter::GasConsumption PathGasMeter::estimateMax(
 
 void PathGasMeter::queue(std::unique_ptr<GasPath>&& _newPath)
 {
-	if (
-		m_highestGasUsagePerJumpdest.count(_newPath->index) &&
-		_newPath->gas < m_highestGasUsagePerJumpdest.at(_newPath->index)
-	)
-		return;
-	m_highestGasUsagePerJumpdest[_newPath->index] = _newPath->gas;
+	auto [it, inserted] = m_highestGasUsagePerJumpdest.emplace(_newPath->index, _newPath->gas);
+	if (!inserted)
+	{
+		if (_newPath->gas < it->second)
+			return;
+		it->second = _newPath->gas;
+	}
 	m_queue[_newPath->index] = std::move(_newPath);
 }
 
@@ -122,8 +123,8 @@ GasMeter::GasConsumption PathGasMeter::handleQueueItem()
 		{
 			auto newPath = std::make_unique<GasPath>();
 			newPath->index = m_items.size();
-			if (m_tagPositions.count(tag))
-				newPath->index = m_tagPositions.at(tag);
+			if (auto it = m_tagPositions.find(tag); it != m_tagPositions.end())
+				newPath->index = it->second;
 			newPath->gas = gas;
 			newPath->largestMemoryAccess = meter.largestMemoryAccess();
 			newPath->state = state->copy();

libevmasm/PathGasMeter.h

@@ -26,7 +26,8 @@
 
 #include <liblangutil/EVMVersion.h>
 
-#include <set>
+#include <unordered_map>
+#include <unordered_set>
 #include <vector>
 #include <memory>
 
@@ -41,7 +42,7 @@ struct GasPath
 	std::shared_ptr<KnownState> state;
 	u256 largestMemoryAccess;
 	GasMeter::GasConsumption gas;
-	std::set<size_t> visitedJumpdests;
+	std::unordered_set<size_t> visitedJumpdests;
 };
 
 /**
@@ -77,7 +78,7 @@ class PathGasMeter
 	/// Map of jumpdest -> gas path, so not really a queue. We only have one queued up
 	/// item per jumpdest, because of the behaviour of `queue` above.
 	std::map<size_t, std::unique_ptr<GasPath>> m_queue;
-	std::map<size_t, GasMeter::GasConsumption> m_highestGasUsagePerJumpdest;
+	std::unordered_map<size_t, GasMeter::GasConsumption> m_highestGasUsagePerJumpdest;
 	std::map<u256, size_t> m_tagPositions;
 	AssemblyItems const& m_items;
 	langutil::EVMVersion m_evmVersion;

libevmasm/PeepholeOptimiser.cpp

@@ -267,11 +267,13 @@ struct SwapComparison: SimplePeepholeOptimizerMethod<SwapComparison>
 
 		if (
 			_swap == Instruction::SWAP1 &&
-			_op.type() == Operation &&
-			swappableOps.count(_op.instruction())
+			_op.type() == Operation
 		)
 		{
-			*_out = swappableOps.at(_op.instruction());
+			auto it = swappableOps.find(_op.instruction());
+			if (it == swappableOps.end())
+				return false;
+			*_out = it->second;
 			return true;
 		}
 		else