Setting an "Asserted" property on xyz.openbmc_project.LED.GroupManager takes a long time

[brandonkimbk]

This is to bring the attention to a problem that was discovered from phosphor-nvme openbmc/phosphor-nvme#4

I found that phosphor-nvme daemon was causing issues with the D-Bus, and realized that while the D-Bus is being stressed, it can take almost 133ms to call a setProperty to xyz.openbmc_project.LED.GroupManager and xyz.openbmc_project.Led.Group for "Asserted" property. This is in contrast to

Looking at the code for the setter, we can see that it should compare and return right away if it's the same

phosphor-led-manager/group.cpp

Lines 12 to 19 in 87fd11c

	bool Group::asserted(bool value)
	{
	// If the value is already what is before, return right away
	if (value ==
	sdbusplus::xyz::openbmc_project::Led::server::Group::asserted())
	{
	return value;
	}

The mitigation for phosphor-nvme was to call the getter manually and not call the setter when the property did not have to change - this decreased the average time by 87% which was much more reasonable.

Why could this be the case? The "setter" in this case "should" just be doing what a "getter" should be doing if the property is the same, however we're seeing a huge performance drop for doing a set.

Workaround:

https://github.com/openbmc/phosphor-nvme/blob/fdffe5c37f0d1feaa90558433f688c3757d2e79a/nvme_manager.cpp#L116-L125

https://github.com/openbmc/phosphor-nvme/blob/fdffe5c37f0d1feaa90558433f688c3757d2e79a/nvme_manager.cpp#L155-L161

[pointbazaar]

Since it's been 3 years, with many changes to the code, is this issue still present?

[williamspatrick]

This seems like something that should be done in the generated sdbusplus bindings rather than sprinkled around each daemon. I'll check on that.