Re: [PATCH 3/3] kunit: factor out str constants from binary assertion structs

[Brendan Higgins]

On Tue, Jan 25, 2022 at 4:00 PM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote:
>
> If the compiler doesn't optimize them away, each kunit assertion (use of
> KUNIT_EXPECT_EQ, etc.) can use 88 bytes of stack space in the worst and
> most common case. This has led to compiler warnings and a suggestion
> from Linus to move data from the structs into static const's where
> possible [1].
>
> This builds upon [2] which did so for the base struct kunit_assert type.
> That only reduced sizeof(struct kunit_binary_assert) from 88 to 64.
>
> Given these are by far the most commonly used asserts, this patch
> factors out the textual representations of the operands and comparator
> into another static const, saving 16 more bytes.
>
> In detail, KUNIT_EXPECT_EQ(test, 2 + 2, 5) yields the following struct
>   (struct kunit_binary_assert) {
>     .assert = <struct kunit_assert>,
>     .operation = "==",
>     .left_text = "2 + 2",
>     .left_value = 4,
>     .right_text = "5",
>     .right_value = 5,
>   }
> After this change
>   static const struct kunit_binary_assert_text __text = {
>     .operation = "==",
>     .left_text = "2 + 2",
>     .right_text = "5",
>   };
>   (struct kunit_binary_assert) {
>     .assert = <struct kunit_assert>,
>     .text = &__text,
>     .left_value = 4,
>     .right_value = 5,
>   }
>
> This also DRYs the code a bit more since these str fields were repeated
> for the string and pointer versions of kunit_binary_assert.
>
> Note: we could name the kunit_binary_assert_text fields left/right
> instead of left_text/right_text. But that would require changing the
> macros a bit since they have args called "left" and "right" which would
> be substituted in `.left = #left` as `.2 + 2 = \"2 + 2\"`.
>
> [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ
> [2] https://lore.kernel.org/linux-kselftest/20220113165931.451305-6-dlatypov@xxxxxxxxxx/
>
> Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx>

Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>