rfnoc: Always retain order of properties

When resolving properties, it is important to retain the order of
properties. In this patch, this is achieved by changing prop_ptrs_t from
being an unordered_set into a vector.

The biggest effect is that node_t::filter_props() now returns filtered
properties in the order they were added.

Resolving properties in the order they were added, and not in any random
order, is very helpful when designing property resolvers. For the MTU
property, however, it is almost always a requirement. That's because the
the MTU property is added first (via noc_block_base), but also, its
resolution already assumes execution of resolvers in a certain order
(see the long comment towards the end of
noc_block_base::noc_block_base()). Execution of resolvers in a certain
order can only be guaranteed if properties are also in a certain order.

A particular issue this resolves is the ability to have properties that
both depend on atomic_item_size and the MTU. Because blocks have no way
of reading the current MTU other than calling
`noc_block_base::get_mtu()`, resolvers requiring access to the MTU
assume that the MTU properties are always resolved first.

The majority of this patch revolves around syntactical differences
between std::vector and std::unordered_set, like using `push_back()`
instead of `insert()` for adding to `prop_ptrs_t`.
This commit is contained in:
Martin Braun 2022-06-13 10:10:53 +02:00 committed by Aaron Rossetto
parent 500b522390
commit 7ca952d914
6 changed files with 31 additions and 19 deletions

View File

@ -216,7 +216,7 @@ protected:
/******************************************
* Internal Registration Functions
******************************************/
using prop_ptrs_t = std::unordered_set<property_base_t*>;
using prop_ptrs_t = std::vector<property_base_t*>;
/*! Register a property for this block
*
@ -501,7 +501,7 @@ private:
for (const auto& type_prop_pair : _props) {
for (const auto& prop : type_prop_pair.second) {
if (predicate(prop)) {
filtered_props.insert(prop);
filtered_props.push_back(prop);
}
}
}

View File

@ -52,7 +52,7 @@ noc_block_base::noc_block_base(make_args_ptr make_args)
prop_ptrs_t tick_rate_prop_refs;
tick_rate_prop_refs.reserve(_tick_rate_props.size());
for (auto& prop : _tick_rate_props) {
tick_rate_prop_refs.insert(&prop);
tick_rate_prop_refs.push_back(&prop);
register_property(&prop);
}
for (auto& prop : _tick_rate_props) {
@ -86,7 +86,7 @@ noc_block_base::noc_block_base(make_args_ptr make_args)
prop_ptrs_t mtu_prop_refs;
mtu_prop_refs.reserve(_mtu_props.size());
for (auto& prop : _mtu_props) {
mtu_prop_refs.insert(&prop);
mtu_prop_refs.push_back(&prop);
register_property(&prop);
}
// If an MTU edge property value changes, this resolver will coerce the
@ -252,6 +252,7 @@ void noc_block_base::set_mtu_forwarding_policy(const forwarding_policy_t policy)
for (auto& prop : _mtu_props) {
const res_source_info src_edge = prop.get_src_info();
prop_ptrs_t output_props{};
output_props.reserve(_mtu_props.size());
for (auto& other_prop : _mtu_props) {
const res_source_info dst_edge = other_prop.get_src_info();
bool add_to_output_props = false;
@ -274,7 +275,7 @@ void noc_block_base::set_mtu_forwarding_policy(const forwarding_policy_t policy)
}
if (add_to_output_props) {
output_props.insert(&other_prop);
output_props.push_back(&other_prop);
}
}

View File

@ -6,6 +6,7 @@
#include <uhd/exception.hpp>
#include <uhd/rfnoc/node.hpp>
#include <uhd/utils/algorithm.hpp>
#include <uhd/utils/log.hpp>
#include <uhdlib/rfnoc/prop_accessor.hpp>
#include <boost/format.hpp>
@ -432,8 +433,8 @@ void node_t::resolve_props()
filter_props([](property_base_t* prop) { return prop->is_dirty(); });
std::list<property_base_t*> all_dirty_props(
initial_dirty_props.cbegin(), initial_dirty_props.cend());
prop_ptrs_t processed_props{};
prop_ptrs_t written_props{};
std::unordered_set<property_base_t*> processed_props{};
std::unordered_set<property_base_t*> written_props{};
RFNOC_LOG_TRACE("Locally resolving " << all_dirty_props.size()
<< " dirty properties plus dependencies.");
@ -448,7 +449,7 @@ void node_t::resolve_props()
for (auto& resolver_tuple : _prop_resolvers) {
auto& inputs = std::get<0>(resolver_tuple);
auto& outputs = std::get<1>(resolver_tuple);
if (!inputs.count(current_input_prop)) {
if (!uhd::has(inputs, current_input_prop)) {
continue;
}
@ -524,10 +525,10 @@ void node_t::forward_edge_property(
// of incoming_prop)
const auto prop_src_type =
res_source_info::invert_edge(incoming_prop->get_src_info().type);
// Set of local properties that match incoming_prop. It can be an empty set,
// or, if the node is misconfigured, a set with more than one entry. Or, if
// all is as expected, it's a set with a single entry.
auto local_prop_set = filter_props(
// List of local properties that match incoming_prop. It can be an empty list,
// or, if the node is misconfigured, have more than one entry. Or, if
// all is as expected, it's a list with a single entry.
auto local_prop_list = filter_props(
[prop_src_type, incoming_prop, incoming_port](property_base_t* prop) -> bool {
return prop->get_src_info().type == prop_src_type
&& prop->get_src_info().instance == incoming_port
@ -535,16 +536,16 @@ void node_t::forward_edge_property(
});
// If there is no such property, we're forwarding a new property
if (local_prop_set.empty()) {
if (local_prop_list.empty()) {
RFNOC_LOG_TRACE(
"Received unknown incoming edge prop: " << incoming_prop->get_id());
local_prop_set.emplace(
local_prop_list.push_back(
inject_edge_property(incoming_prop, {prop_src_type, incoming_port}));
}
// There must be either zero results, or one
UHD_ASSERT_THROW(local_prop_set.size() == 1);
UHD_ASSERT_THROW(local_prop_list.size() == 1);
auto local_prop = *local_prop_set.begin();
auto local_prop = *local_prop_list.begin();
prop_accessor_t prop_accessor{};
prop_accessor.forward<false>(incoming_prop, local_prop);

View File

@ -67,8 +67,9 @@ rfnoc_rx_streamer::rfnoc_rx_streamer(const size_t num_chans,
for (size_t i = 0; i < num_chans; i++) {
prop_ptrs_t mtu_resolver_out;
mtu_resolver_out.reserve(_mtu_in.size());
for (auto& mtu_prop : _mtu_in) {
mtu_resolver_out.insert(&mtu_prop);
mtu_resolver_out.push_back(&mtu_prop);
}
add_property_resolver({&_mtu_in[i]},

View File

@ -55,8 +55,9 @@ rfnoc_tx_streamer::rfnoc_tx_streamer(const size_t num_chans,
for (size_t i = 0; i < num_chans; i++) {
prop_ptrs_t mtu_resolver_out;
mtu_resolver_out.reserve(_mtu_out.size());
for (auto& mtu_prop : _mtu_out) {
mtu_resolver_out.insert(&mtu_prop);
mtu_resolver_out.push_back(&mtu_prop);
}
add_property_resolver({&_mtu_out[i]},

View File

@ -17,6 +17,8 @@ public:
test_node_t(size_t num_inputs, size_t num_outputs)
: _num_input_ports(num_inputs), _num_output_ports(num_outputs)
{
// Intentionally register props in a different order than they are
// declared to verify order of filter_props() is what we want it to be
register_property(&_double_prop_user, [this]() {
std::cout << "Calling clean callback for user prop" << std::endl;
this->user_prop_cb_called = true;
@ -77,7 +79,6 @@ public:
bool user_prop_cb_called = false;
private:
property_t<double> _double_prop_user{"double_prop", 0.0, {res_source_info::USER}};
property_t<double> _double_prop_in{
"double_prop", 0.0, {res_source_info::INPUT_EDGE, 0}};
property_t<double> _double_prop_out{
@ -86,6 +87,7 @@ private:
"multi_instance_prop", 0.0, {res_source_info::USER, 0}};
property_t<double> multi_instance_prop_1{
"multi_instance_prop", 0.0, {res_source_info::USER, 1}};
property_t<double> _double_prop_user{"double_prop", 0.0, {res_source_info::USER}};
const size_t _num_input_ports;
const size_t _num_output_ports;
@ -152,6 +154,12 @@ BOOST_AUTO_TEST_CASE(test_node_accessor)
});
BOOST_CHECK_EQUAL(user_props.size(), 3);
// We choose an iterator here so we can verify the order of user_props
// independent of the data type being used underneath
auto upit = user_props.begin();
BOOST_CHECK_EQUAL((*upit++)->get_id(), "double_prop");
BOOST_CHECK_EQUAL((*upit++)->get_id(), "multi_instance_prop");
BOOST_CHECK_EQUAL((*upit++)->get_id(), "multi_instance_prop");
std::map<std::string, int> prop_count;
for (const auto& prop : user_props) {
prop_count[prop->get_id()]++;