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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-02-23 14:36, Jastrzebski, MichalX K:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2015-02-20 15:46, Jastrzebski, MichalX K:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > > > 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.
> > > > >
> > > > > 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
> > > >
> > > > 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
> > >
> > > Hi Neil,
> > >
> > > I think this library offers something quite useful probably not for everyone,
> > > but for many people that use DPDK, and it is measuring quite accurately,
> > > how many spare cycles a CPU have after executing any serial tasks (as you
> > will know).
> > > If you look at two places in example application: main_loop()
> > > and l2fwd_fwd() functions, you will see two possible approach there, but
> > > this is not limited to that. You can even nest headroom objects and
> > measure
> > > process time of particular packets type.
> > > Of course, this will add an overhead due to the measurements,
> > > but that time is also measured, so any user can know what is the relative
> > > time "wasted" for measuring all this.
> > > If time delays are measured in bigger timestamps, are handled reliably,
> > > the cost of measuring will be low.
> > > I find this quite similar to the power library case. I would say that library is
> > not useful
> > > for every application, but there are several cases where it can be
> > > (as demonstrated with l3fwd-power app).
> > >
> > > About your last bit, not sure if I understood it right, but in case of the
> > included sample app,
> > > the main measurement to see if we are overusing a CPU is the packet count
> > > in a queue (in this case RX queue), and I believe this should be used for
> > other apps,
> > > especially in those that use a pipeline model, where queues and rings are
> > the key part.
> > >
> > > As a final point, last week (12th of February), there was a request for a
> > tool/library like this
> > > from a user in the mailing list (Ilan Borenshtein), which indicates that this
> > would be useful
> > > (probably not just for him, but for others). It probably could be achieved by
> > the user
> > > by adding their own code, but I believe this library would be a good-to-
> > have,
> > > in case a user is looking for an easy way to calculate the exposed above.
> > > Let us give the users an example of this method and we will expand it with
> > more
> > > advanced application that may show capabilities of dynamic load scaling
> > based on headroom library measurement.
> > 
> Hi Thomas,
> 
> > I wonder how this library is related to DPDK.
> > I'm not against its integration, though the question must be asked.
> > DPDK is a set of libraries. What kind of library fit with DPDK goals and
> > deserve to be integrated?
> > 
> I think this library fits into dpdk goals, because it is simple and optimized for fast packet processing.

I don't have a strong opinion here. If anyone else wants to comment, please speak now.

> The library provides an easy way for existing DPDK users to modify their applications to measure available CPU headroom.
> If we integrate it, users will verify it and decide what else they need and we will implement this.

Do you mean that you plan to add some features to this library?
Is it going to stay at providing some stats or could you make some actions
like time-sharing helpers?

> > I don't know whether it's related but nobody acknowledged this patchset.
> We were waiting for Neil's final comments. He did not mention anything else since the last time. 
> When Pawel sends the next version with the copyright date corrected, Pablo will ack this.
> 
> > I also feel that the name of this library is a bit too vague. Some people
> > were asking first what means "headroom". It's actually for CPU headroom
> > monitoring.
> > What about "cpuheadroom", "cpuheadroomstat", "jobstat"?
> 
> I think we can change the name to "cpuheadroom" as it describes more clear this library.

If you're focusing on CPU usage with possible actions, yes.
If you're focusing on decision helper, jobstat would be better IMHO.

> > Last comment, less important: as many of your colleagues, you don't pay
> > attention to the copyright dates. I'm pretty sure this code was not written
> > in 2010. So why claiming it?
> Sorry, we will fix this of course and pay more attention now.

OK, thanks