Re: [PATCH] mm/zsmalloc: inline obj_malloc() and obj_free()

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (26/11/05 10:39), Jenhao Chen wrote:
> On (26/11/05 14:23), Sergey Senozhatsky wrote:
> > I suppose there is performance benefit?
> > 
> > I guess I don't mind in principle, one thing to note is that this conflicts
> > with [1].
> > 
> > [1] https://lore.kernel.org/linux-kernel/20261105044752.1997895-1-senozhatsky@chromium.org
> 
> Hi Sergey,

Hello,

> Yes -- in a steady-state microbenchmark of paired alloc+write and free
> operations across 8 compressed object size classes (384..3136 B):
> 
> 1. On current mm-new (zs_malloc + zs_obj_write + zs_free):
>    - Clang 22 -O2: -7.7 (arm64) / -11.8 (x86_64) retired instructions
>      per pair (-3.2% cycles on x86_64).
>    - Clang 22 -O2 + CONFIG_AUTOFDO_CLANG: -13.0 (arm64) / -17.5 (x86_64)
>      retired instructions per pair (-4.8% cycles on x86_64), while
>      shrinking zs_malloc() from 2,872 B to 1,828 B on arm64 (-36%) and
>      2,088 B to 1,474 B on x86_64 (-29%).
>    - GCC 15 -O2 (x86_64, without AutoFDO): GCC leaves both obj_malloc()
>      (306 B) and obj_free() (120 B) out-of-line while inlining the
>      single-caller slow path alloc_zspage() into zs_malloc(); the patch
>      saves -23.7 retired instructions per pair and shrinks zs_malloc()
>      from 1,738 B to 1,441 B (-17%).

Cool.

> 2. Rebased on top of [1] (zs_store + zs_free):
>    In [PATCH 6/7], the new static obj_write() helper has two call sites
>    in zs_store(), so both Clang 22 -O2 (360 B on arm64, 219 B on x86_64)
>    and GCC 15 -O2 (obj_write.isra.0, 229 B on x86_64) leave obj_write()
>    out-of-line as well. In the zs_store() fast path, obj_malloc() packs
>    (m_zpdesc, obj, zspage->class) via location_to_obj(), and the
>    out-of-line obj_write() immediately unpacks obj_to_location(),
>    recomputes offset_in_page(class->size * obj_idx), and re-checks
>    !ZsHugePage(zspage).
> 
>    If we also mark obj_write() __always_inline alongside obj_malloc()
>    and obj_free() (with noinline on alloc_zspage()), the compiler CSEs
>    those redundant operations across the fast path:
>    - arm64 (Clang 22): -31.3 (-O2, -4.0%) / -34.7 (-O2 + AUTOFDO, -4.5%)
>      retired instructions per zs_store() + zs_free() pair (vs. -8.4 for
>      obj_malloc() alone), shrinking zs_store() under AUTOFDO from
>      3,144 B to 2,448 B (-22%).
>    - x86_64 (Clang 22): -26.6 (-O2) / -31.8 (-O2 + AUTOFDO) retired
>      instructions per pair (~4.0-4.5% latency reduction).
>    - x86_64 (GCC 15 -O2): -37.1 retired instructions (-5.1%) and -5.3%
>      latency (-5.5% cycles) per pair, while keeping zs_store() slightly
>      smaller (1,997 B -> 1,943 B).

Great, I'd love to see that in v2 commit message.

> Would you prefer that I send v2 rebased on top of [1] (including
> __always_inline on obj_write()), or send v2 against current mm-new
> first?

If it doesn't trouble you too much, can we finalize [1] first so then
you can send your patch atop of it?  I can ping you when [1] is done.