unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: DJ Delorie <dj@redhat.com>
To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Cc: carlos@redhat.com, libc-alpha@sourceware.org, nd@arm.com
Subject: Re: [PATCH] Add malloc micro benchmark
Date: Mon, 18 Dec 2017 18:02:10 -0500	[thread overview]
Message-ID: <xnr2rrabd9.fsf@greed.delorie.com> (raw)
In-Reply-To: <DB6PR0801MB20538A414F1659748E703918833C0@DB6PR0801MB2053.eurprd08.prod.outlook.com> (message from Wilco Dijkstra on Mon, 18 Dec 2017 15:18:47 +0000)


Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Since DJ didn't seem keen on increasing the tcache size despite it
> showing major gains across a wide range of benchmarks,

It's not that I'm not keen on increasing the size, it's that there are
drawbacks to doing so and I don't want to base such a change on a guess
(even a good guess).  If you have benchmarks, let's collect them and add
them to the trace corpus.  I can send you my corpus.  (We don't have a
good solution for centrally storing such a corpus, yet) Let's run all
the tests against all the options and make an informed decision, that's
all.  If it shows gains for synthetic benchmarks, but makes qemu slower,
we need to know that.

Also, as Carlos noted, there are some downstream uses where a larger
cache may be detrimental.  Sometimes there are no universally "better"
defaults, and we provide tunables for those cases.

And, as always, I can be out-voted if the consensus disagrees with me ;-)

> I decided to fix the performance for the single-threaded case at
> least. It's now 2.5x faster on a few sever benchmarks (of course the
> next question is whether tcache is actually useful in its current
> form).

Again, tcache is intended to help the multi-threaded case.  Your patches
help the single-threaded case.  If you recall, I ran your patch against
my corpus of multi-threaded tests, and saw no regressions, which is
good.

So our paranoia here is twofold...

1. Make sure that when someone says "some benchmarks" we have those
   benchmarks available to us, either as a microbenchmark in glibc or as
   a trace we can simulate and benchmark.  No more random benchmarks! :-)

2. When we say a patch "is faster", let's run all our benchmarks and
   make sure that we don't mean "on some benchmarks."  The whole point
   of the trace/sim stuff is to make sure key downstream users aren't
   left out of the optimization work, and end up with worse performance.

We probably should add "on all major architectures" too but that assumes
we have machines on which we can run the benchmarks.

So we should be able to answer your question, not just wonder...

> I'd have to check how easy it is to force it to use the thread arena.

I'm guessing we could have a glibc-internal API to tag the heap as
"corrupt" which would preclude using it.

> If consolidation doesn't work that's a serious bug.

Sometimes it's not a case of "doesn't work" as a case of "not attempted
for performance reasons".  If we can show that a different design choice
is universally better[*], we should change it.

[*] or at least, universally-enough for a "system" allocator like glibc
    must provide.


  parent reply	other threads:[~2017-12-18 23:00 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-01 13:51 [PATCH] Add malloc micro benchmark Wilco Dijkstra
2017-12-01 16:13 ` Carlos O'Donell
2017-12-18 15:18   ` Wilco Dijkstra
2017-12-18 16:32     ` Carlos O'Donell
2018-01-02 18:20       ` [PATCH v2] " Wilco Dijkstra
2018-01-02 18:45         ` DJ Delorie
2018-01-03 12:12           ` Wilco Dijkstra
2018-01-03 15:07             ` Carlos O'Donell
2018-01-04 13:48               ` Wilco Dijkstra
2018-01-04 16:37                 ` Adhemerval Zanella
2018-01-05 14:32                 ` Carlos O'Donell
2018-01-05 15:50                   ` Adhemerval Zanella
2018-01-05 16:17                     ` Carlos O'Donell
2018-01-05 16:46                       ` Adhemerval Zanella
2018-01-05 17:27                         ` Carlos O'Donell
2018-01-05 14:33         ` Carlos O'Donell
2018-01-05 16:28           ` Joseph Myers
2018-01-05 17:26             ` Carlos O'Donell
2018-02-28 12:40               ` Florian Weimer
2018-02-28 14:11                 ` Ondřej Bílka
2018-02-28 14:16                   ` Florian Weimer
2018-02-28 16:16                     ` Carlos O'Donell
2018-02-28 20:17                       ` Ondřej Bílka
2018-02-28 16:46                     ` Ondřej Bílka
2018-02-28 17:01                       ` Wilco Dijkstra
2018-02-28 18:21                         ` Carlos O'Donell
2018-02-28 19:56                         ` Ondřej Bílka
2018-02-28 21:56                           ` DJ Delorie
2018-03-01 11:24                             ` Ondřej Bílka
2017-12-18 23:02     ` DJ Delorie [this message]
2017-12-28 14:09       ` [PATCH] " Wilco Dijkstra
2017-12-28 19:01         ` DJ Delorie
  -- strict thread matches above, loose matches on Subject: below --
2019-02-01 16:27 Wilco Dijkstra
2019-02-08 19:37 ` DJ Delorie
2019-02-14 16:38   ` Wilco Dijkstra
2019-02-14 20:42     ` DJ Delorie
2019-02-28  4:52 ` Carlos O'Donell
2019-03-04 17:35   ` Wilco Dijkstra
2019-03-18 17:16     ` Wilco Dijkstra
2019-04-09  5:25       ` Carlos O'Donell

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

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xnr2rrabd9.fsf@greed.delorie.com \
    --to=dj@redhat.com \
    --cc=Wilco.Dijkstra@arm.com \
    --cc=carlos@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=nd@arm.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 read-only IMAP folder(s) and NNTP newsgroup(s).