> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Qiu, Michael
> Sent: Wednesday, December 10, 2014 10:59 AM
> To: Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Avoid possible memory cpoy when sort hugepages
>
> On 12/10/2014 6:41 PM, Richardson, Bruce wrote:
> > On Wed, Dec 10, 2014 at 10:25:41AM +0800, Michael Qiu wrote:
> >> When the first address is the compared address in the loop,
> >> it will also do memory copy, which is meaningless,
> >> worse more, when hugepg_tbl is mostly in order. This should
> >> be a big deal in large hugepage memory systerm(like hunderd
> >> or thousand GB).
> > I actually doubt the time taken by this sorting is a significant part of the
> > initialization time taken, even for hundreds of GBs of memory. Do you have
> > any measurements to confirm this?
> > [However, that's not to say that we can't merge in this patch]
>
> I've no hardware env of so huge memory, so I haven't do measurements on
> this.
>
> This is not a big improvement, but indeed it may do lots of useless
> memory copy in initialize stat.
>
> It could, at least could save time :)
I wonder why we do need to write our own bubble sort procedure?
Why we can't use standard qsort() here?
Konstantin
>
> Thanks,
> Michael
> >
> >
> >> Meanwhile smallest_idx never be a value of -1,so remove this check.
> >>
> >> This patch also includes some coding style fix.
> >>
> >> Signed-off-by: Michael Qiu <michael.qiu at intel.com>
> >> ---
> >> lib/librte_eal/linuxapp/eal/eal_memory.c | 13 +++++--------
> >> 1 file changed, 5 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
> >> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> >> index e6cb919..700aba2 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> >> @@ -678,14 +678,13 @@ error:
> >> static int
> >> sort_by_physaddr(struct hugepage_file *hugepg_tbl, struct hugepage_info
> >> *hpi)
> >> {
> >> - unsigned i, j;
> >> - int compare_idx;
> >> + unsigned i, j, compare_idx;
> >> uint64_t compare_addr;
> >> struct hugepage_file tmp;
> >>
> >> for (i = 0; i < hpi->num_pages[0]; i++) {
> >> compare_addr = 0;
> >> - compare_idx = -1;
> >> + compare_idx = i;
> >>
> >> /*
> >> * browse all entries starting at 'i', and find the
> >> @@ -704,11 +703,9 @@ sort_by_physaddr(struct hugepage_file *hugepg_tbl,
> >> struct hugepage_info *hpi)
> >> }
> >> }
> >>
> >> - /* should not happen */
> >> - if (compare_idx == -1) {
> >> - RTE_LOG(ERR, EAL, "%s(): error in physaddr sorting\n",
> >> __func__);
> >> - return -1;
> >> - }
> >> + /* avoid memory copy when the first entry is the compared */
> >> + if (compare_idx == i)
> >> + continue;
> >>
> >> /* swap the 2 entries in the table */
> >> memcpy(&tmp, &hugepg_tbl[compare_idx],
> >> --
> >> 1.9.3
> >>
