Re: ping: [patch] malloc per-thread cache ready for review

[Will Newton <will dot newton at gmail dot com>]

On Thu, Mar 2, 2017 at 5:43 PM, DJ Delorie <dj@redhat.com> wrote:
> Will Newton <will.newton@gmail.com> writes:
>> As such it basically involves diffing the the head of the branch
>> against the last merge from master and reviewing that diff rather than
>> reviewing a sequence of self-contained patches.
>
> I've been told you can do anything with git... but that doesn't mean I
> know how, or that others do ;-) I attached the patch to the original
> mail for that reason, which included everything but the </<= fix.

The total patch isn't actually as many lines as I thought it might be
so its still pretty manageable.

>>  - There are some inconsistencies with whitespace around operators.
>
> Examples?  Carlos went over whitespace in a pre-review...

+# define MAX_TCACHE_SIZE       (MALLOC_ALIGNMENT * 63)
+# define TCACHE_IDX            ((MAX_TCACHE_SIZE / MALLOC_ALIGNMENT) + 1)
+# define size2tidx_(bytes)     (((bytes) + MALLOC_ALIGNMENT - 1) /
MALLOC_ALIGNMENT)
+
+# define tidx2csize(idx)       ((idx)*MALLOC_ALIGNMENT + SIZE_SZ)
+# define tidx2usize(idx)       ((idx)*MALLOC_ALIGNMENT)
+
+/* When "x" is a user-provided size.  */
+# define usize2tidx(x) size2tidx_(x)
+/* When "x" is from chunksize().  */
+# define csize2tidx(x) size2tidx_((x)-SIZE_SZ)

The first few have spaces around operators but some of the later ones do not.

>>  - MAYBE_INIT_TCACHE could be defined to be empty when !USE_TCACHE,
>> this would reduce #ifdefs
>
> I'll look into that (and the other suggestion), thanks!
>
>> Is there a reasonably straightforward way to reproduce (a subset of)
>> the benchmark results you referenced?
>
> If you check out the dj/malloc branch, it includes a workload simulator
> (and the tools for making workloads) with built-in benchmarking code.  I
> have the workloads, and they're shareable (but huge), I just haven't
> figured out a good place to store them for public consumption yet.
>
> Note that the simulator is "just a program" and should run under any
> glibc (or other environment, including other mallocs), only the tracing
> needs a specific glibc.

Thanks, I'll check it out.