linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: Timofey Titovets <nefelim4ag@gmail.com>
Cc: linux-mm@kvack.org
Subject: Re: [PATCH] KSM: Replace jhash2 with xxhash
Date: Thu, 21 Sep 2017 15:06:59 -0700	[thread overview]
Message-ID: <20170921220659.GI4311@tassilo.jf.intel.com> (raw)
In-Reply-To: <CAGqmi76=ntcE5tvYGKOQynpTfUfUotwXZQuU4iUC+H_6rua7Yw@mail.gmail.com>

On Fri, Sep 22, 2017 at 12:37:25AM +0300, Timofey Titovets wrote:
> With defaults:
> jhash2: ~4.7%
> xxhash64: ~3.3%
> 
> 3.3/4.7 ~= 0.7 -> Profit: ~30%
> 11/18   ~= 0.6 -> Profit: ~40%
> (if i calculate correctly of course)

Sounds good.

Please add all performance information to the changelog.

> 
> >> >> @@ -51,6 +52,12 @@
> >> >>  #define DO_NUMA(x)   do { } while (0)
> >> >>  #endif
> >> >>
> >> >> +#if BITS_PER_LONG == 64
> >> >> +typedef      u64     xxhash;
> >> >> +#else
> >> >> +typedef      u32     xxhash;
> >> >> +#endif
> >> >
> >> > This should be in xxhash.h ?
> >>
> >> This is a "hack", for compile time chose appropriate hash function.
> >> xxhash ported from upstream code,
> >> upstream version don't do that (IMHO), as this useless in most cases.
> >> That only can be useful for memory only hashes.
> >> Because for persistent data it's obvious to always use one hash type 32/64.
> >
> > I don't think it's a hack. It makes sense. Just should be done centrally
> > in Linux, not in a specific user.
> 
> So, i must add separate patch for xxhash.h?

Yes.

> If yes, may be you can suggest which list must be in copy?
> (i can't find any info about maintainers of ./lib/ in MAINTAINERS)

Just copy linux-kernel. It would be all merged together.

> If we decide to patch xxhash.h,
> may be that will be better to wrap above if-else by something like:
> /*
>  * Only for in memory use
>  */
> xxhash_t xxhash(const void *input, size_t length, uint64_t seed);

Yes that's fine.

-Andi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-09-21 22:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-21  7:45 [PATCH] KSM: Replace jhash2 with xxhash Timofey Titovets
2017-09-21 15:36 ` Andi Kleen
2017-09-21 17:35   ` Timofey Titovets
2017-09-21 20:05     ` Andi Kleen
2017-09-21 21:37       ` Timofey Titovets
2017-09-21 22:06         ` Andi Kleen [this message]
2017-09-22  8:44 ` Christian Borntraeger

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=20170921220659.GI4311@tassilo.jf.intel.com \
    --to=ak@linux.intel.com \
    --cc=linux-mm@kvack.org \
    --cc=nefelim4ag@gmail.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).