From a62634717e02c3d0514f2acb3882cc83687ee0ee Mon Sep 17 00:00:00 2001 From: Jake Turner Date: Tue, 13 Feb 2024 13:56:02 +0000 Subject: [PATCH] Vulkan BDA debugging force m_BufferAddresses to be sorted The implementation usage of m_BufferAddresses assumes the container is sorted and requires the use of lower_bound() API Added rdcsortedflatmap subclass of rdcflatmap Only expose upper_bound(), lower_bound() APIs in rdcsortedflatmap The upper/lower bound APIs require a sorted container --- renderdoc/api/replay/rdcflatmap.h | 144 ++++++++++++++++--------- renderdoc/core/intervals.h | 2 +- renderdoc/driver/vulkan/vk_info.h | 2 +- renderdoc/replay/basic_types_tests.cpp | 31 +++--- 4 files changed, 111 insertions(+), 68 deletions(-) diff --git a/renderdoc/api/replay/rdcflatmap.h b/renderdoc/api/replay/rdcflatmap.h index c94f9c317..d486c002a 100644 --- a/renderdoc/api/replay/rdcflatmap.h +++ b/renderdoc/api/replay/rdcflatmap.h @@ -1,3 +1,27 @@ +/****************************************************************************** + * The MIT License (MIT) + * + * Copyright (c) 2019-2024 Baldur Karlsson + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + ******************************************************************************/ + #pragma once #include @@ -126,35 +150,6 @@ struct rdcflatmap return {(begin() + idx), inserted}; } - iterator upper_bound(const Key &key) - { - if(!sorted) - sort(); - - size_t idx = lower_bound_idx(key); - - // almost the same behaviour as lower_bound, except if we actually have the key, return the next - // element. - if(idx < size() && storage.at(idx).first == key) - return begin() + idx + 1; - - return begin() + idx; - } - - const_iterator upper_bound(const Key &key) const - { - size_t idx = lower_bound_idx(key); - - // almost the same behaviour as lower_bound, except if we actually have the key, return the next - // element. - if(idx < size() && storage.at(idx).first == key) - return begin() + idx + 1; - - return begin() + idx; - } - - iterator lower_bound(const Key &key) { return begin() + lower_bound_idx(key); } - const_iterator lower_bound(const Key &key) const { return begin() + lower_bound_idx(key); } iterator begin() { return storage.begin(); } iterator end() { return storage.end(); } const_iterator begin() const { return storage.begin(); } @@ -167,8 +162,33 @@ struct rdcflatmap storage.swap(other.storage); } void clear() { storage.clear(); } -private: +protected: rdcarray> storage; + + size_t lower_bound_idx(const Key &id) const + { + // start looking at the whole range + size_t start = 0, sz = size(); + // continue iterating until the range is empty + while(sz > 0) + { + const size_t halfsz = sz / 2; + const size_t mid = start + halfsz; + const Key comp = storage.at(mid).first; + if(comp < id) + { + start = mid + 1; + sz -= halfsz + 1; + } + else + { + sz = halfsz; + } + } + return start; + } + +private: bool sorted = (SortThreshold == 0); void sort() @@ -216,29 +236,6 @@ private: return (begin() + idx)->second; } - size_t lower_bound_idx(const Key &id) const - { - // start looking at the whole range - size_t start = 0, sz = size(); - // continue iterating until the range is empty - while(sz > 0) - { - const size_t halfsz = sz / 2; - const size_t mid = start + halfsz; - const Key comp = storage.at(mid).first; - if(comp < id) - { - start = mid + 1; - sz -= halfsz + 1; - } - else - { - sz = halfsz; - } - } - return start; - } - iterator unsorted_find(const Key &id) { for(auto it = begin(); it != end(); ++it) @@ -277,3 +274,44 @@ private: return storage.back().second; } }; + +// a version of rdcflatmap which is guaranteed to be sorted +// adds upper_bound and lower_bound APIs +template +struct rdcsortedflatmap : rdcflatmap +{ + using iterator = rdcpair *; + using const_iterator = const rdcpair *; + + using rdcflatmap::begin; + using rdcflatmap::size; + using rdcflatmap::storage; + using rdcflatmap::lower_bound_idx; + + iterator upper_bound(const Key &key) + { + size_t idx = lower_bound_idx(key); + + // almost the same behaviour as lower_bound, except if we actually have the key, return the next + // element. + if(idx < size() && storage.at(idx).first == key) + return begin() + idx + 1; + + return begin() + idx; + } + + const_iterator upper_bound(const Key &key) const + { + size_t idx = lower_bound_idx(key); + + // almost the same behaviour as lower_bound, except if we actually have the key, return the next + // element. + if(idx < size() && storage.at(idx).first == key) + return begin() + idx + 1; + + return begin() + idx; + } + + iterator lower_bound(const Key &key) { return begin() + lower_bound_idx(key); } + const_iterator lower_bound(const Key &key) const { return begin() + lower_bound_idx(key); } +}; diff --git a/renderdoc/core/intervals.h b/renderdoc/core/intervals.h index ab6f94251..65c2ee081 100644 --- a/renderdoc/core/intervals.h +++ b/renderdoc/core/intervals.h @@ -162,7 +162,7 @@ template struct Intervals { public: - using MapType = rdcflatmap; + using MapType = rdcsortedflatmap; typedef IntervalRef interval; typedef IntervalsIter iterator; diff --git a/renderdoc/driver/vulkan/vk_info.h b/renderdoc/driver/vulkan/vk_info.h index a8f92be00..b85552d29 100644 --- a/renderdoc/driver/vulkan/vk_info.h +++ b/renderdoc/driver/vulkan/vk_info.h @@ -626,7 +626,7 @@ struct VulkanCreationInfo VkMemoryRequirements mrq; }; std::unordered_map m_Buffer; - rdcflatmap m_BufferAddresses; + rdcsortedflatmap m_BufferAddresses; struct BufferView { diff --git a/renderdoc/replay/basic_types_tests.cpp b/renderdoc/replay/basic_types_tests.cpp index 3c03d2c41..57f7a653b 100644 --- a/renderdoc/replay/basic_types_tests.cpp +++ b/renderdoc/replay/basic_types_tests.cpp @@ -2243,10 +2243,23 @@ TEST_CASE("Test flatmap type", "[basictypes][flatmap]") CHECK(test.find(5)->second == "foo"); }; + SECTION("empty_map") + { + rdcflatmap unsorted; + CHECK(unsorted.begin() == unsorted.end()); + CHECK(unsorted.find(0) == unsorted.end()); + + rdcflatmap sorted; + CHECK(sorted.begin() == sorted.end()); + CHECK(sorted.find(0) == sorted.end()); + } +} + +TEST_CASE("Test sorted flatmap type", "[basictypes][sortedflatmap]") +{ SECTION("upper_bound") { - // set SortThreshold to 0 to force sorted semantics always - rdcflatmap test; + rdcsortedflatmap test; test[5] = "foo"; test[7] = "bar"; @@ -2292,17 +2305,9 @@ TEST_CASE("Test flatmap type", "[basictypes][flatmap]") SECTION("empty_map") { - rdcflatmap unsorted; - CHECK(unsorted.begin() == unsorted.end()); - CHECK(unsorted.find(0) == unsorted.end()); - CHECK(unsorted.lower_bound(1) == 0); - CHECK(unsorted.upper_bound(2) == 0); - - rdcflatmap sorted; - CHECK(sorted.begin() == sorted.end()); - CHECK(sorted.find(0) == sorted.end()); - CHECK(sorted.lower_bound(1) == 0); - CHECK(sorted.upper_bound(2) == 0); + rdcsortedflatmap sortedflatmap; + CHECK(sortedflatmap.lower_bound(1) == 0); + CHECK(sortedflatmap.upper_bound(2) == 0); } };