From: Neil Horman <nhorman@redhat.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: Neil Horman <nhorman@tuxdriver.com>,
Thomas Monjalon <thomas.monjalon@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Mcnamara, John" <john.mcnamara@intel.com>
Subject: Re: [dpdk-dev] [PATCH] validate_abi: build faster by augmenting make with job count
Date: Mon, 1 Aug 2016 14:08:55 -0400 [thread overview]
Message-ID: <20160801180855.GA5220@hmsreliant.think-freely.org> (raw)
In-Reply-To: <749B7847-05FA-4502-AAAA-11C863831E6E@intel.com>
On Mon, Aug 01, 2016 at 04:16:12PM +0000, Wiles, Keith wrote:
>
> >> Neil
> >>
> > Sorry for the delayed response, I've been on vacation.
> >
> >> Your modified copy of make has no bearing on the topic we are taking about customers using dpdk in standard distros right?
> >>
> > !? I really don't know what you're saying here. My only reason for commenting
> > on my copy of make was to consider an explination for why make -j 0 might be
> > working for me, and not for you. I've since uninstalled and resinalled make on
> > my system, and my copy now behaves as yours does
> >
> > But thats all really irrelevant. I don't know what you mean by this not having
> > any bearing on the conversation since we're talking about customers using dpdk
> > in a distro. We're not really talking about that at all (if we were using make
> > would be a moot point, since distro users tend to only use pre-compiled
> > binaries). Were talking about accelerating the build process when comparing
> > ABIs on two different dpdk versions, thats all.
>
> Neil, (I am trying to not read your style of text as condescending and I will try to not do that as well)
>
> Not everyone uses DPDK from prebuilt libraries and we need to support them as well, correct?
>
Correct, which is why I didn't understand your initial comment:
"Your modified copy of make has no bearing on the topic we are taking about
customers using dpdk in standard distros right?"
I read that as you saying that the topic we are discussing here is DPDK use in
standard distros, to which I am replying "No, that is not what we are talking
about here at all"
Standard Distros, as I am involved with them, are standard because the end user
typically strives to never build software included in the distro themselves. As
such, this patch has no bearing whatsoever on those end users, because they
expect to just use a pre-built binary that conforms to a given ABI level. They
have no need for this code
As you note, of course other upstream developers don't use pre-built binaries,
and that is who this change is targeting
> >
> >> Seems odd to me to send this out with 0 or lspci as it may fail because of no lspci and will fail on all Ubuntu systems.
> >>
> > Again, don't really understand what your saying. If you look at the patch,
> > neither of your assertions are true. With this patch and no other change, the
> > validate_abi script behaves exactly as it does now. The only thing I've done is
> > add a check for the DPDK_MAKE_JOBS environment variable, and if its not set,
> > either:
> >
> > a) Set DPDK_MAKE_JOBS to 1 if lscpu doesn't exist on the system
> > b) Set DPDK_MAKE_JOBS to the number of online cpus if lscpu does exist
> >
> > All of that gets overridden if you manually set DPDK_MAKE_JOBS to something
> > else.
> >
> > seems pretty straightforward to me.
>
> At this point do we need to add yet another environment variable to get the correct behavior with DPDK. DPDK is very simple to build today and I worry we keep adding special variables to build DPDK. Can we just use a cleaner default, then adding more and more special build requirements? Adding this one is fine, but it also means the customer must read the docs to find this new variable.
>
Please read back through the thread. The DPDK_MAKE_JOBS environment variable
was specifically used because it already exists in the build (see
test-build.sh). Thomas specifically asked me to change the environment variable
name so that it can be resued. We're not adding anything here that isn't
already there in other locations.
> DPDK should be build able with the least amount of docs to read, then they can read the docs more later. Just looking at how the developer can get started building DPDK without RTFM problem. At some point they need to read the docs to possibly runs the examples, but to build DPDK should very simple IMO.
So, this script has nothing to do with actually building the DPDK, only
analyzing differences in ABI between two arbitrary levels. Nothing about this
change makes building the DPDK harder, easier, or in any way different.
>
> >
> >> If we ship with 1 then why even bother the adding code and if I have to edit the file or some other method to get better compile performance then why bother as well.
> >>
> > Please stop and think for a minute. Why would you need to edit a file to change
> > anything? If lscpu exists, then everything happens automatically. If it
> > doesn't you can still just run:
> > export DPDK_MAKE_JOBS=$BIGNUMBER; validate_abi.sh <args>
>
> Please do not add extra environment variable we would start to get to the point of having so many pre-build requirements it becomes the private knowledge to just build DPDK or a huge setup/RTFM problem.
>
See above, I'm getting the impression that you're just arguing without actually
looking at the code first.
> >
> > and it works fine. If ubuntu has some other utiilty besides lscpu to parse cpu
> > counts, fine, we can add that in as a feature, but I don't see why that should
> > stop other, non-ubuntu systems from taking advantage of this.
> >
> >> Setting the value to some large number does not make any sense to me and if I have to edit file every time or maintain a patch just seems silly.
Its no different than setting -j without an argument. -j with no argument,
allows make to just fork jobs as deeply as the dependency graph allows (above
and beyond the number of cpus that can run them). This can lead to large run
queues on the scheduler (depending on the amount of parallelism the dependency
graph evaluates to). While thats not necessecarily a bad thing, it may not be
what a developer wants (as it shows up as a large load value). Regardless,
setting -j to any number larger than the maximum number of independent jobs that
a given make file can find is equivalent to setting -j with no argument. Thats
all I'm getting at here, export DPDK_MAKE_JOBS=<large number> is exactly
equivalent to -j without an argument for <large number> >= <max number of
parallel jobs possible>. So you can get the behavior you want, or something
more restrictive with my patch. If we just set -j in the validate abi script
without an argument, then we only get one of those possibilities.
If your argument is that no one would want to set anything other than maximal
job count, I would respond by saying that I like to do that frequently, because
it lets me limit the load of a build that I run, and it lets me serialize make
in the event that I want to debug a build problem. Allowing reduced job counts
is a valuable and relevant feature.
> >>
> > Goodness Keith, stop for just a minute with the editing the file train your on.
> > Its an environment variable, you don't have to edit a file to change it.
>
> Yes Neil, you also need to stop an think about what you are placing on the developer to build DPDK. This one little problem is not the real issue to me, but a symptom of a growing problem in DPDK around how DPDK is build and the amount of knowledge or setup it requires to do this one simple task.
>
Keith, I feel like you may be missing the point of this script. This is the 3rd
time you've asserted that this change makes DPDK harder to build. I assert
that:
a) The validate_abi script is in no way required to build the DPDK (i.e. it is
an ancilliary tool that compares ABI at two different arbitrary points in the
project git history. It simply does so by building the DPDK as part of its
work)
b) The environment variable in question is already being used by other scripts
(i.e. there is nothing new being added here in terms of environment variables,
only reuse of existing ones)
c) Completely ignoring this environment variable in no way impacts behavior
(i.e. failure to set this environment variable attempts to accelerate build
parallelism within the confines of this script, if the right tools are
available, and leaves the behavior unchanged otherwise)
Can you please elaborate on how you feel this change makes DPDK harder to build?
> >
> >> It just seems easier to set it to -j and not use lspci at all. This way we all win as I am not following your logic at all.
> >>
> > This doesn't even make any sense. If you set it to -j then you get make issuing
> > jobs at the max rate make can issue them, which is potentially fine, but may not
> > be what developers want in the event they don't want this script to monopolize
> > their system. The way its written now lets people get reasonable behavior in
> > most cases, and opt-in to the extreeme case should they desire. That makes far
> > more sense to me, then just chewing up as much cpu as possible all the time.
>
> I only suggest -j as this would give the developer the best build performance without having to require lscpu or setting up yet another build environment variable. The lscpu command does not exist on all systems today and other non-Linux based systems in the future.
>
In your environment it does yes, but that may not be the only thing people want,
thats what I'm saying, and I think thats worth taking into account.
Also, fwiw, lscpu should be part of every standard linux distribution, as its
part of the utils-linux package, which contains a number of core utilities to
make user space usable:
http://packages.ubuntu.com/precise/amd64/util-linux/filelist
I'm not sure if you are asserting that it doesn't exist on your ubuntu system,
but that seems to suggest that you've somehow removed it.
> The amount of gain with -j over with -j is a reasonable performance option, as for chewing up cpu performance for 20-50 seconds is not a problem IMO. Please look at the bigger picture and not just your way of building DPDK as most will have a standalone machine as a development platform (I would assume) and utilizing that machine to the fullest is not a problem (unless you need to get the last digit of PI in another process :-).
>
Again, this utility builds the dpdk as part of its work, but is not used for the
DPDK build process itself. That seems to be a critical difference in my mind
here.
> I have DPDK building on Mac OS and lscpu does not exist for that system (I do not know about windows). Think about the future some and using -j just seems to have the least amount of requirements on the system, right?
>
I'm sorry, no. From what I can see we currently support linux and bsd on a
variety of hardware architectures. Thats what we're targeting. I don't think
its reasonable to say no to a change because it uses a tool that may not exist
on a future operating system, that no one is targeting at the moment, or plans
to (Note also, that this script will work just fine on those systems, it will
just use the basic behavior of -j 1 because the if [ -e /usr/bin/lscpu ] test
will fail, which is more than I can say for the cpu_layout.py script, which will
traceback when reading /proc/cpuinfo or dpdk_nic_bind, which will simply fail
when trying to bind or unbind a nic using vfio on those systems)
I think thats really where were falling apart here. You seem to be under the
impression that this script is somehow tied to the build process, and by making
this change I am somehow negatively impacting that process (or potentially
breaking it). Thats simply not true. This script is self contained, and is in
no way part of the build system. The script itself just happens to build the
dpdk as part of its work internally), but a developer never has to use it to
acutally build the dpdk itself.
> If the developer does not like the ‘chew up all of my CPUs' problem then he can read the docs to set the environment variable, but I suspect that would not even happen in 99% of the cases.
Ah, so we agree in priciple, and we're just haggling over price? :)
I would counter your argument with the fact that 99% of developers also have a
system which contains lscpu, and so they will get the right amount of
parallelism for their system, not a flood of parallel jobs, nor a serialized
single job environment. Its the happy medium I'm after, not too much, not too
little.
Neil
>
> Keith
>
next prev parent reply other threads:[~2016-08-01 18:08 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-20 17:09 Neil Horman
2016-07-20 17:40 ` Thomas Monjalon
2016-07-20 17:48 ` Neil Horman
2016-07-20 19:47 ` Wiles, Keith
2016-07-20 20:15 ` Thomas Monjalon
2016-07-20 20:16 ` Neil Horman
2016-07-20 22:32 ` Wiles, Keith
2016-07-21 13:54 ` Neil Horman
2016-07-21 14:09 ` Wiles, Keith
2016-07-21 15:06 ` Neil Horman
2016-07-21 15:22 ` Wiles, Keith
2016-07-21 18:34 ` Neil Horman
2016-07-24 18:08 ` Wiles, Keith
2016-08-01 11:49 ` Neil Horman
2016-08-01 16:16 ` Wiles, Keith
2016-08-01 18:08 ` Neil Horman [this message]
2016-07-20 19:02 ` [dpdk-dev] [PATCH v2] " Neil Horman
2016-07-22 10:46 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160801180855.GA5220@hmsreliant.think-freely.org \
--to=nhorman@redhat.com \
--cc=dev@dpdk.org \
--cc=john.mcnamara@intel.com \
--cc=keith.wiles@intel.com \
--cc=nhorman@tuxdriver.com \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).