From 948001f39f016de043e8cb59165015347b1a34bc Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Tue, 25 Nov 2025 00:47:11 +0000 Subject: [PATCH] Merge the two autoidx hashtables into one When something calls `IdString::c_str()` on an autoidx ID, we need to cache the full string in a thread-safe way. If we need to allocate an entry in some data structure to do that, it's difficult to do in a thread-safe no-performance-hazard way. So instead, store the cached string pointer in the same hashtable as the prefix pointer. --- kernel/rtlil.cc | 19 ++++++++----------- kernel/rtlil.h | 36 ++++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 9477b04a8..6077237ac 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -38,8 +38,7 @@ bool RTLIL::IdString::destruct_guard_ok = false; RTLIL::IdString::destruct_guard_t RTLIL::IdString::destruct_guard; std::vector RTLIL::IdString::global_id_storage_; std::unordered_map RTLIL::IdString::global_id_index_; -std::unordered_map RTLIL::IdString::global_autoidx_id_prefix_storage_; -std::unordered_map RTLIL::IdString::global_autoidx_id_storage_; +std::unordered_map RTLIL::IdString::global_autoidx_id_storage_; std::unordered_map RTLIL::IdString::global_refcount_storage_; std::vector RTLIL::IdString::global_free_idx_list_; @@ -93,8 +92,9 @@ int RTLIL::IdString::really_insert(std::string_view p, std::unordered_map p_autoidx = parse_autoidx(p.substr(autoidx_pos)); if (p_autoidx.has_value()) { - auto prefix_it = global_autoidx_id_prefix_storage_.find(-*p_autoidx); - if (prefix_it != global_autoidx_id_prefix_storage_.end() && p.substr(0, autoidx_pos) == *prefix_it->second) + auto autoidx_it = global_autoidx_id_storage_.find(-*p_autoidx); + if (autoidx_it != global_autoidx_id_storage_.end() && + p.substr(0, autoidx_pos) == *autoidx_it->second.prefix) return -*p_autoidx; // Ensure NEW_ID/NEW_ID_SUFFIX will not create collisions with the ID // we're about to create. @@ -267,7 +267,7 @@ void RTLIL::OwningIdString::collect_garbage() global_free_idx_list_.push_back(i); } - for (auto it = global_autoidx_id_prefix_storage_.begin(); it != global_autoidx_id_prefix_storage_.end();) { + for (auto it = global_autoidx_id_storage_.begin(); it != global_autoidx_id_storage_.end();) { if (collector.live.find(it->first) != collector.live.end()) { ++it; continue; @@ -276,13 +276,10 @@ void RTLIL::OwningIdString::collect_garbage() ++it; continue; } - auto str_it = global_autoidx_id_storage_.find(it->first); - if (str_it != global_autoidx_id_storage_.end()) { - delete[] str_it->second; - global_autoidx_id_storage_.erase(str_it); - } - it = global_autoidx_id_prefix_storage_.erase(it); + delete[] it->second.full_str; + it = global_autoidx_id_storage_.erase(it); } + int64_t time_ns = PerformanceTimer::query() - start; Pass::subtract_from_current_runtime_ns(time_ns); gc_ns += time_ns; diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 29eac294f..06c38c52e 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -134,6 +134,13 @@ struct RTLIL::IdString std::string_view str_view() const { return {buf, static_cast(size)}; } }; + struct AutoidxStorage { + // Append the negated (i.e. positive) ID to this string to get + // the real string. The prefix strings must live forever. + const std::string *prefix; + // Cache of the full string, or nullptr if not cached yet. + char *full_str; + }; // the global id string cache @@ -147,12 +154,9 @@ struct RTLIL::IdString static std::vector global_id_storage_; // Lookup table for non-autoidx IDs static std::unordered_map global_id_index_; - // Shared prefix string storage for autoidx IDs, which have negative - // indices. Append the negated (i.e. positive) ID to this string to get - // the real string. The prefix strings must live forever. - static std::unordered_map global_autoidx_id_prefix_storage_; - // Explicit string storage for autoidx IDs - static std::unordered_map global_autoidx_id_storage_; + // Storage for autoidx IDs, which have negative indices, i.e. all entries in this + // map have negative keys. + static std::unordered_map global_autoidx_id_storage_; // All (index, refcount) pairs in this map have refcount > 0. static std::unordered_map global_refcount_storage_; static std::vector global_free_idx_list_; @@ -205,7 +209,7 @@ struct RTLIL::IdString static IdString new_autoidx_with_prefix(const std::string *prefix) { log_assert(!Multithreading::active()); int index = -(autoidx++); - global_autoidx_id_prefix_storage_.insert({index, prefix}); + global_autoidx_id_storage_.insert({index, {prefix, nullptr}}); return from_index(index); } @@ -238,16 +242,16 @@ struct RTLIL::IdString inline const char *c_str() const { if (index_ >= 0) return global_id_storage_.at(index_).buf; - auto it = global_autoidx_id_storage_.find(index_); - if (it != global_autoidx_id_storage_.end()) - return it->second; - const std::string &prefix = *global_autoidx_id_prefix_storage_.at(index_); + AutoidxStorage &s = global_autoidx_id_storage_.at(index_); + if (s.full_str != nullptr) + return s.full_str; + const std::string &prefix = *s.prefix; std::string suffix = std::to_string(-index_); char *c = new char[prefix.size() + suffix.size() + 1]; memcpy(c, prefix.data(), prefix.size()); memcpy(c + prefix.size(), suffix.c_str(), suffix.size() + 1); - global_autoidx_id_storage_.insert(it, {index_, c}); + s.full_str = c; return c; } @@ -262,7 +266,7 @@ struct RTLIL::IdString *out += global_id_storage_.at(index_).str_view(); return; } - *out += *global_autoidx_id_prefix_storage_.at(index_); + *out += *global_autoidx_id_storage_.at(index_).prefix; *out += std::to_string(-index_); } @@ -348,7 +352,7 @@ struct RTLIL::IdString if (index_ >= 0) { return const_iterator(global_id_storage_.at(index_)); } - return const_iterator(global_autoidx_id_prefix_storage_.at(index_), -index_); + return const_iterator(global_autoidx_id_storage_.at(index_).prefix, -index_); } const_iterator end() const { return const_iterator(); @@ -358,7 +362,7 @@ struct RTLIL::IdString if (index_ >= 0) { return Substrings(global_id_storage_.at(index_)); } - return Substrings(global_autoidx_id_prefix_storage_.at(index_), -index_); + return Substrings(global_autoidx_id_storage_.at(index_).prefix, -index_); } inline bool lt_by_name(const IdString &rhs) const { @@ -411,7 +415,7 @@ struct RTLIL::IdString #endif return *(storage.buf + i); } - const std::string &id_start = *global_autoidx_id_prefix_storage_.at(index_); + const std::string &id_start = *global_autoidx_id_storage_.at(index_).prefix; if (i < id_start.size()) return id_start[i]; i -= id_start.size();