All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Latypov <dlatypov@google.com>
To: brendanhiggins@google.com, davidgow@google.com
Cc: linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-kselftest@vger.kernel.org, skhan@linuxfoundation.org,
	torvalds@linux-foundation.org,
	Daniel Latypov <dlatypov@google.com>
Subject: [PATCH v3 0/6] kunit: refactor assertions to use less stack
Date: Thu, 13 Jan 2022 08:59:25 -0800	[thread overview]
Message-ID: <20220113165931.451305-1-dlatypov@google.com> (raw)

*** BLURB HERE ***
Every KUNIT_ASSERT/EXPECT() invocation puts a `kunit_assert` object onto
the stack. The most common one is `kunit_binary_assert` which is 88
bytes on UML. So in the cases where the compiler doesn't optimize this
away, we can very quickly blow up the stack size.

This series implements Linus' suggestion in [1].
Namely, we split out the file, line number, and assert_type
(EXPECT/ASSERT) out of kunit_assert.

We can also drop the entirely unused `struct kunit *test` field, saving
a bit more space as well.

All together, sizeof(struct kunit_assert) went from 48 to 24 on UML.
Note: the other assert types are bigger, see [2].

This series also adds in an example test that uses all the base
KUNIT_EXPECT macros to both advertise their existence to new users and
serve as a smoketest for all these changes here.

[1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ
[2] e.g. consider the most commonly used assert (also the biggest)
  struct kunit_binary_assert {
          struct kunit_assert assert;
          const char *operation;
          const char *left_text;
          long long left_value;
          const char *right_text;
          long long right_value;
  };
So sizeof(struct kunit_binary_assert) = went from 88 to 64.
I.e. only a 27% reduction instead of 50% in the most common case.

All 3 of the `const char*` could be split out into a `static` var as well,
but that's a bit trickier to do with how all the macros are written.

=== Changelog ===
v1 -> v2:
* made the new example test more focused on documenting the macros
rather than using them all as a smoketest
* s/kunit_failed_assertion()/kunit_do_failed_assertion()
* added `unlikely()` to `if(!(pass))` check in KUNIT_ASSERTION()

v2 -> v3:
* elaborate on intermediate TODO in patch 5 (deleted in patch 6)
* update with more Reviewed-by's


Daniel Latypov (6):
  kunit: add example test case showing off all the expect macros
  kunit: move check if assertion passed into the macros
  kunit: drop unused kunit* field in kunit_assert
  kunit: factor out kunit_base_assert_format() call into kunit_fail()
  kunit: split out part of kunit_assert into a static const
  kunit: drop unused assert_type from kunit_assert and clean up macros

 include/kunit/assert.h         | 88 +++++++++++-----------------------
 include/kunit/test.h           | 53 ++++++++++----------
 lib/kunit/assert.c             | 15 ++----
 lib/kunit/kunit-example-test.c | 42 ++++++++++++++++
 lib/kunit/test.c               | 27 +++++------
 5 files changed, 117 insertions(+), 108 deletions(-)


base-commit: ad659ccb5412874c6a89d3588cb18857c00e9d0f
-- 
2.34.1.703.g22d0c6ccf7-goog


             reply	other threads:[~2022-01-13 16:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 16:59 Daniel Latypov [this message]
2022-01-13 16:59 ` [PATCH v3 1/6] kunit: add example test case showing off all the expect macros Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 2/6] kunit: move check if assertion passed into the macros Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 3/6] kunit: drop unused kunit* field in kunit_assert Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 4/6] kunit: factor out kunit_base_assert_format() call into kunit_fail() Daniel Latypov
2022-01-21 23:01   ` Brendan Higgins
2022-01-13 16:59 ` [PATCH v3 5/6] kunit: split out part of kunit_assert into a static const Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 6/6] kunit: drop unused assert_type from kunit_assert and clean up macros Daniel Latypov

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=20220113165931.451305-1-dlatypov@google.com \
    --to=dlatypov@google.com \
    --cc=brendanhiggins@google.com \
    --cc=davidgow@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.