Skip to content

Commit

Permalink
Remove redundant calls to get child scheduler group during initializa…
Browse files Browse the repository at this point in the history
…tion (sonic-net#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 <stephens@nvidia.com>
  • Loading branch information
stephenxs authored Nov 17, 2021
1 parent 18ea840 commit 103fdf0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
26 changes: 20 additions & 6 deletions orchagent/qosorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::vector<sai_object_id_t>>(groups_count)
.child_groups = std::vector<std::vector<sai_object_id_t>>(groups_count),
.group_has_been_initialized = std::vector<bool>(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)
Expand All @@ -1250,14 +1261,17 @@ sai_object_id_t QosOrch::getSchedulerGroup(const Port &port, const sai_object_id
}

uint32_t child_count = attr.value.u32;
vector<sai_object_id_t> 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)
{
continue;
}

vector<sai_object_id_t> 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;
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions orchagent/qosorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ class QosOrch : public Orch
{
std::vector<sai_object_id_t> groups;
std::vector<std::vector<sai_object_id_t>> child_groups;
std::vector<bool> group_has_been_initialized;
};

std::unordered_map<sai_object_id_t, SchedulerGroupPortInfo_t> m_scheduler_group_port_info;
Expand Down

0 comments on commit 103fdf0

Please sign in to comment.