Re: [PATCH v2] pipeline: add statistics for librte_pipeline ports and tables

On Wed, 20 May 2015 16:44:35 +0200
Thomas Monjalon [off-list ref] wrote:

Please Cristian, do not top post.
I'm replacing your comment in the right context.

2015-05-20 13:57, Dumitrescu, Cristian:

quoted

From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]

quoted

Thanks for the detailed explanation.

You are raising a trade-off problem about
feature/maintenance/performance.
I think we must choose among 4 solutions:
	1/ always enabled
	2/ build-time log level
	3/ run-time option
	4/ build-time option

It's good to have this discussion and let other contributors giving their
opinion.

2015-05-19 22:41, Dumitrescu, Cristian:

quoted

1. What is the technical solution to avoid performance loss due to stats
support?
Generally, I would agree with you that config options should be avoided,
especially those that alter the API (function prototypes, data structure
layouts, etc). Please note this is not the case for any of our patches,
as only the stats collection is enabled/disabled, while the data
structures and functions are not changed by the build time option.

But what can you do for statistics? We all know that collecting the stats
sucks up CPU cycles, especially due to memory accesses, so stats always
have a run-time cost. Traditionally, stats are typically enabled for
debugging purposes and then turned off for the release version when
performance is required. How can this be done if build time flags are not
used?

Statistics in drivers are always enabled (first solution).
If those statistics are only used for debugging, why not using the build-time
option CONFIG_RTE_LOG_LEVEL? (second solution)

Can you please describe what solution 2 on your list (build-time log
level) consists of?

I see log level useful for printing messages when an event takes place,
but this is not what these stats patches are about. We want to poll
for those counters on demand: if the build-time flag is off, then the
value of those counters is 0; when the build-time is on, then the stats
counters have the real value. Basically, the build-time flag only
enables/disables the update of the counters at run-time, which is where
CPU cycles are consumed. I am not sure how the log levels can help here?

I think that counting stats is a kind of logging.
Some stats are always counted (see drivers) and you want to use these ones
only for debugging (after rebuilding DPDK with some debug options).
So I suggest, as second solution, to check CONFIG_RTE_LOG_LEVEL is at debug
level instead of having one option per module.
It would be implemented with "#if RTE_LOG_LEVEL == RTE_LOG_DEBUG" in
RTE_PIPELINE_STATS_ADD.

In my experience, per-queue or per-cpu statistics have no visible performance
impact. And every real world deployment wants statistics