Re: [dpdk-dev] [PATCH v4 0/6] update jhash function

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, May 13, 2015 at 01:52:33PM +0000, De Lara Guarch, Pablo wrote:
> Hi Neil,
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Tuesday, May 12, 2015 4:33 PM
> > To: De Lara Guarch, Pablo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v4 0/6] update jhash function
> > 
> > On Tue, May 12, 2015 at 12:02:32PM +0100, Pablo de Lara wrote:
> > > Jenkins hash function was developed originally in 1996,
> > > and was integrated in first versions of DPDK.
> > > The function has been improved in 2006,
> > > achieving up to 60% better performance, compared to the original one.
> > >
> > > This patchset updates the current jhash in DPDK,
> > > including two new functions that generate two hashes from a single key.
> > >
> > > It also separates the existing hash function performance tests to
> > > another file, to make it quicker to run.
> > >
> > > changes in v4:
> > > - Simplify key alignment checks
> > > - Include missing x86 arch check
> > >
> > > changes in v3:
> > >
> > > - Update rte_jhash_1word, rte_jhash_2words and rte_jhash_3words
> > >   functions
> > >
> > > changes in v2:
> > >
> > > - Split single commit in three commits, one that updates the existing
> > functions
> > >   and another that adds two new functions and use one of those functions
> > >   as a base to be called by the other ones.
> > > - Remove some unnecessary ifdefs in the code.
> > > - Add new macros to help on the reutilization of constants
> > > - Separate hash function performance tests to another file
> > >   and improve cycle measurements.
> > > - Rename existing function rte_jhash2 to rte_jhash_32b
> > >   (something more meaninful) and mark rte_jhash2 as
> > >   deprecated
> > >
> > > Pablo de Lara (6):
> > >   test/hash: move hash function perf tests to separate file
> > >   test/hash: improve accuracy on cycle measurements
> > >   hash: update jhash function with the latest available
> > >   hash: add two new functions to jhash library
> > >   hash: remove duplicated code
> > >   hash: rename rte_jhash2 to rte_jhash_32b
> > >
> > >  app/test/Makefile               |    1 +
> > >  app/test/test_func_reentrancy.c |    2 +-
> > >  app/test/test_hash.c            |    4 +-
> > >  app/test/test_hash_func_perf.c  |  145 +++++++++++++++++
> > >  app/test/test_hash_perf.c       |   71 +--------
> > >  lib/librte_hash/rte_jhash.h     |  338 +++++++++++++++++++++++++++++-
> > ---------
> > >  6 files changed, 402 insertions(+), 159 deletions(-)
> > >  create mode 100644 app/test/test_hash_func_perf.c
> > >
> > > --
> > > 1.7.4.1
> > >
> > >
> > did you run this through the ABI checker?  I see you're removing several
> > symbols
> > that will likely need to go through the ABI deprecation process.
> > 
> > Neil
> 
> I had not run it, but I just did. I see no problems on librte_hash
> (but I see some on rte_ethdev.h, due to another commit).
> 
> Anyway, I renamed two functions to be more meaningful, but those functions are "static inline", 
> so I am not sure exactly what the deprecation process is for those.
> What I did was leaving the original function that calls the same function as the new renamed one,
> but adds a line warning that the functions is deprecated.
> 
> Is that OK or should I do it differently?
> 
As long as their all static inline and binaries that are already compiled can
continue to access the data structures they reference at the member offsets
encoded to them at compile time, you should be ok.

Thanks!
Neil

> Thanks!
> Pablo
>