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>
next prev parent 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).