From 103fdf0551921d90350852de963d527d54d41f71 Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Wed, 17 Nov 2021 14:12:03 +0800 Subject: [PATCH] Remove redundant calls to get child scheduler group during initialization (#1965) - What I did The QoS orchagent calls SAI APIs to get the number of child scheduler groups and then initialize them. After that, the size of the child scheduler groups vector will be non-zero, which indicates the child scheduler groups have been initialized and prevents QoS orchagent from calling SAI get APIs again. However, on some platforms, it may be that some of the scheduler groups don't have child groups, leaving size of child scheduler groups always being zero. It causes QoS orchagent to call the SAI get API each time the scheduler group is handled, which wastes a lot of time, especially during fast reboot. An extra flag indicating whether the child groups have been initialized is introduced to avoid redundant calls. - Why I did it To optimize QoS orchagent performance during initialization especially for fast reboot. - How I verified it It can be covered by the existing regression test. - Details if related I did a pair of tests, comparing the time to handle scheduler and child scheduler groups between the old and the new (optimized) version. Dump and sairedis.record attached. Signed-off-by: Stephen Sun --- orchagent/qosorch.cpp | 26 ++++++++++++++++++++------ orchagent/qosorch.h | 1 + 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/orchagent/qosorch.cpp b/orchagent/qosorch.cpp index 59cb801bb4aa..0431b369d318 100644 --- a/orchagent/qosorch.cpp +++ b/orchagent/qosorch.cpp @@ -1225,18 +1225,29 @@ sai_object_id_t QosOrch::getSchedulerGroup(const Port &port, const sai_object_id m_scheduler_group_port_info[port.m_port_id] = { .groups = std::move(groups), - .child_groups = std::vector>(groups_count) + .child_groups = std::vector>(groups_count), + .group_has_been_initialized = std::vector(groups_count) }; - } + + SWSS_LOG_INFO("Port %s has been initialized with %u group(s)", port.m_alias.c_str(), groups_count); + } /* Lookup groups to which queue belongs */ - const auto& groups = m_scheduler_group_port_info[port.m_port_id].groups; + auto& scheduler_group_port_info = m_scheduler_group_port_info[port.m_port_id]; + const auto& groups = scheduler_group_port_info.groups; for (uint32_t ii = 0; ii < groups.size() ; ii++) { const auto& group_id = groups[ii]; - const auto& child_groups_per_group = m_scheduler_group_port_info[port.m_port_id].child_groups[ii]; + const auto& child_groups_per_group = scheduler_group_port_info.child_groups[ii]; if (child_groups_per_group.empty()) { + if (scheduler_group_port_info.group_has_been_initialized[ii]) + { + // skip this iteration if it has been initialized which means there're no children in this group + SWSS_LOG_INFO("No child group for port %s group 0x%" PRIx64 ", skip", port.m_alias.c_str(), group_id); + continue; + } + attr.id = SAI_SCHEDULER_GROUP_ATTR_CHILD_COUNT;//Number of queues/groups childs added to scheduler group sai_status = sai_scheduler_group_api->get_scheduler_group_attribute(group_id, 1, &attr); if (SAI_STATUS_SUCCESS != sai_status) @@ -1250,7 +1261,9 @@ sai_object_id_t QosOrch::getSchedulerGroup(const Port &port, const sai_object_id } uint32_t child_count = attr.value.u32; - vector child_groups(child_count); + + SWSS_LOG_INFO("Port %s group 0x%" PRIx64 " has been initialized with %u child group(s)", port.m_alias.c_str(), group_id, child_count); + scheduler_group_port_info.group_has_been_initialized[ii] = true; // skip this iteration if there're no children in this group if (child_count == 0) @@ -1258,6 +1271,7 @@ sai_object_id_t QosOrch::getSchedulerGroup(const Port &port, const sai_object_id continue; } + vector child_groups(child_count); attr.id = SAI_SCHEDULER_GROUP_ATTR_CHILD_LIST; attr.value.objlist.list = child_groups.data(); attr.value.objlist.count = child_count; @@ -1272,7 +1286,7 @@ sai_object_id_t QosOrch::getSchedulerGroup(const Port &port, const sai_object_id } } - m_scheduler_group_port_info[port.m_port_id].child_groups[ii] = std::move(child_groups); + scheduler_group_port_info.child_groups[ii] = std::move(child_groups); } for (const auto& child_group_id: child_groups_per_group) diff --git a/orchagent/qosorch.h b/orchagent/qosorch.h index 5e8f41e7c68e..69e7cef70b2e 100644 --- a/orchagent/qosorch.h +++ b/orchagent/qosorch.h @@ -189,6 +189,7 @@ class QosOrch : public Orch { std::vector groups; std::vector> child_groups; + std::vector group_has_been_initialized; }; std::unordered_map m_scheduler_group_port_info;