Re: [dpdk-dev] [PATCH v5 0/3] new headroom stats library and example application

[Neil Horman <nhorman@tuxdriver.com>]

On Thu, Feb 19, 2015 at 01:18:41PM +0100, Pawel Wodkowski wrote:
> Hi community,
> I would like to introduce library for measuring load of some arbitrary jobs. It
> can be used to profile every kind of job sets on any arbitrary execution unit or
> tasking library.
> 
> In provided l2fwd-headroom example I demonstrate how to use this library to
> select optimal rx burst poll time. Jobs are selected by using existing rte_timer
> library calls. This example does no limit possible schemes on which this library
> can be used.
> 
> PATCH v5 changes:
>  - Fix spelling and checkpatch.pl errors.
>  - Add maintainer claim for library and example app.
> 
> PATCH v4 changes:
>  - use proper branch for generating patch.
> 
> PATCH v3 changes:
>  - Fix spelling.
> 
> PATCH v2 changes:
>  - Remove jobs management/callback from library to not duplicate tasking library
>    behaviour.
>  - Cleenup/remove useless statistics.
>  - Rework example application to use rte_timer library for jobs selection.
>  - Introduce new app parameter '-l' for automatic thousands separating in stats.
>  - More readable statistics format.
> 
> 
> 
> Pawel Wodkowski (3):
>   librte_headroom: New library for checking core/system/app load
>   examples: introduce new l2fwd-headroom example
>   MAINTAINERS: claim responsibility for headroom library and example app
> 
>  MAINTAINERS                                  |    4 +
>  config/common_bsdapp                         |    5 +
>  config/common_linuxapp                       |    5 +
>  examples/Makefile                            |    1 +
>  examples/l2fwd-headroom/Makefile             |   51 ++
>  examples/l2fwd-headroom/main.c               | 1040 ++++++++++++++++++++++++++
>  lib/Makefile                                 |    1 +
>  lib/librte_headroom/Makefile                 |   54 ++
>  lib/librte_headroom/rte_headroom.c           |  271 +++++++
>  lib/librte_headroom/rte_headroom.h           |  324 ++++++++
>  lib/librte_headroom/rte_headroom_version.map |   19 +
>  mk/rte.app.mk                                |    4 +
>  12 files changed, 1779 insertions(+)
>  create mode 100644 examples/l2fwd-headroom/Makefile
>  create mode 100644 examples/l2fwd-headroom/main.c
>  create mode 100644 lib/librte_headroom/Makefile
>  create mode 100644 lib/librte_headroom/rte_headroom.c
>  create mode 100644 lib/librte_headroom/rte_headroom.h
>  create mode 100644 lib/librte_headroom/rte_headroom_version.map
> 
> -- 
> 1.9.1
> 
> 
I'm sorry but I still fail to see how this is a particularly useful library.  It
clearly works fine, but it composes an application event loop in its own terms,
and measures stats based on that.  While thats ok, any application is already
going to have to write its own event loop, and can makethe same measurements
synchnously within that loop, using alot less code to optimize its polling time. 

In other words, I think this is one of those cases where this library is
probably somewhat useful for anyone who just wants to write an application in
terms the semantics exposed by this library, but not at all useful for anyone
else.  I'd personally rather not have the extra code to maintain here.

Stephen just gave a presentation at netdev about some of the performance
optimization measurements Brocade did with DPDK and how they fine tuned their
environment.  One of the big take aways for me was that making time based
measurements (especially if it was using the tsc), created cpu stalls that
skewed the measurements, and so the best optimizations they made avoided time
measurements, opting instead for packet count metrics.

Neil