treewide: rework struct gpiod_line_bulk
Subsystems (2)
PATCH v3 1/1 treewide: rework struct gpiod_line_bulk
From: Bartosz Golaszewski <[email protected]>
Current implementation of struct gpiod_line_bulk uses stack memory excessively. The structure is big: it's an array 64 pointers + 4 bytes size. That amounts to 260 bytes on 32-bit and 516 bytes on 64-bit architectures respectively. It's also used everywhere as all functions dealing with single lines eventually end up calling bulk counterparts.
The rework addresses it by making the bulk structure opaque and providing appropriate interfaces for library users while retaining a way for internal users to allocate single line bulks on the stack.
The macro-based loop has been removed. In its place we provide a function iterating over all lines held by a bulk and calling the provided callback function for each line.
Since bulk operations can now fail, a bunch of test-cases has been added to cover the relevant code.
While at it: using the word offset both when referring to line's HW offset in a chip as well as the offset in a bulk leads to confusion. This patch renames the bulk offset to index.
Some additional improvements to the patch by:
Kent Gibson <[email protected]>
---
v1 -> v2:
- make offset -> index in all bulk-related interfaces and docs
- drop the owner chip reference from struct gpiod_line_bulk and reuse the owner of the first line
- drop bulk_same_chip helper
v2 -> v3:
- drop bulk iterators in favor of looping over bulk indices
bindings/cxx/gpiod.hpp | 20 ++- bindings/cxx/line_bulk.cpp | 89 ++++++------- bindings/python/gpiodmodule.c | 169 ++++++++++++++++--------- include/gpiod.h | 143 +++++++++------------ lib/core.c | 211 +++++++++++++++++++------------ lib/ctxless.c | 53 +++++--- lib/helpers.c | 61 +++++---- tests/Makefile.am | 1 + tests/gpiod-test.h | 2 + tests/tests-bulk.c | 158 ++++++++++++++++++++++++ tests/tests-chip.c | 59 +++++---- tests/tests-event.c | 73 +++++++---- tests/tests-line.c | 226 +++++++++++++--------------------- 13 files changed, 759 insertions(+), 506 deletions(-) create mode 100644 tests/tests-bulk.c
bindings/cxx/gpiod.hpp +15 -5
@@ -626,18 +626,20 @@ public: /** * @brief Get the line at given offset. - * @param offset Offset of the line to get. + * @param index Index of the line to get. * @return Reference to the line object. + * @note This method will throw if index is equal or greater than the + * number of lines currently held by this bulk. */ - GPIOD_API line& get(unsigned int offset); + GPIOD_API line& get(unsigned int index); /** * @brief Get the line at given offset without bounds checking. - * @param offset Offset of the line to get. + * @param index Offset of the line to get. * @return Reference to the line object. * @note No bounds checking is performed. */ - GPIOD_API line& operator[](unsigned int offset); + GPIOD_API line& operator[](unsigned int index); /** * @brief Get the number of lines currently held by this object.
@@ -839,8 +841,16 @@ public: private: + struct line_bulk_deleter + { + void operator()(::gpiod_line_bulk *bulk); + }; + void throw_if_empty(void) const; - void to_line_bulk(::gpiod_line_bulk* bulk) const; + + using line_bulk_ptr = ::std::unique_ptr<::gpiod_line_bulk, line_bulk_deleter>; + + line_bulk_ptr to_line_bulk(void) const; ::std::vector<line> _m_bulk; };
bindings/cxx/line_bulk.cpp +40 -49
206 lines not on this page · raw message
bindings/python/gpiodmodule.c +113 -56
380 lines not on this page · raw message
include/gpiod.h +60 -83
208 lines not on this page · raw message
lib/core.c +132 -79
380 lines not on this page · raw message
lib/ctxless.c +36 -17
156 lines not on this page · raw message
lib/helpers.c +40 -21
106 lines not on this page · raw message
tests/Makefile.am +1 -0
@@ -22,6 +22,7 @@ bin_PROGRAMS = gpiod-test gpiod_test_SOURCES = \ gpiod-test.c \ gpiod-test.h \ + tests-bulk.c \ tests-chip.c \ tests-ctxless.c \ tests-event.c \
tests/gpiod-test.h +2 -0
@@ -24,10 +24,12 @@* regular 'struct typename' syntax. */ typedef struct gpiod_chip gpiod_chip_struct ; +typedef struct gpiod_line_bulk gpiod_line_bulk_struct ;typedef struct gpiod_chip_iter gpiod_chip_iter_struct ;typedef struct gpiod_line_iter gpiod_line_iter_struct ;G_DEFINE_AUTOPTR_CLEANUP_FUNC (gpiod_chip_struct, gpiod_chip_close); +G_DEFINE_AUTOPTR_CLEANUP_FUNC (gpiod_line_bulk_struct, gpiod_line_bulk_free);G_DEFINE_AUTOPTR_CLEANUP_FUNC (gpiod_chip_iter_struct, gpiod_chip_iter_free);G_DEFINE_AUTOPTR_CLEANUP_FUNC (gpiod_line_iter_struct, gpiod_line_iter_free);
tests/tests-bulk.c new file +158 -0
158 lines not on this page · raw message
tests/tests-chip.c +29 -30
114 lines not on this page · raw message
tests/tests-event.c +47 -26
202 lines not on this page · raw message
tests/tests-line.c +86 -140
416 lines not on this page · raw message
Signature · 1 line
2.29.1
Quoted 35 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Current implementation of struct
gpiod_line_bulkuses stack memory excessively. The structure is big: it's an array 64 pointers + 4 bytes size. That amounts to 260 bytes on 32-bit and 516 bytes on 64-bit architectures respectively. It's also used everywhere as all functions dealing with single lines eventually end up calling bulk counterparts.The rework addresses it by making the bulk structure opaque and providing appropriate interfaces for library users while retaining a way for internal users to allocate single line bulks on the stack.
The macro-based loop has been removed. In its place we provide a function iterating over all lines held by a bulk and calling the provided callback function for each line.
Since bulk operations can now fail, a bunch of test-cases has been added to cover the relevant code.
While at it: using the word offset both when referring to line's HW offset in a chip as well as the offset in a bulk leads to confusion. This patch renames the bulk offset to index.
Some additional improvements to the patch by:
Kent Gibson <[email protected]>Signed-off-by Bartosz Golaszewski <[email protected]>---
v1 -> v2:
- make offset -> index in all bulk-related interfaces and docs
- drop the owner chip reference from structgpiod_line_bulkand reuse the owner of the first line
- dropbulk_same_chiphelper
Quoted 3 lines
v2 -> v3:
- drop bulk iterators in favor of looping over bulk indices
Applied this patch. We can add some tweaks later.
Bartosz