Skip to content

Commit f7c4fe6

Browse files
Nicolas Pitrecarlescufi
authored andcommitted
shell: optimize history storage a bit
When padding is added to "fill" the gap in order to wrap to the beginning of the ring buffer, such padding is attributed to the current item being added. Let's attribute it to the previous entry instead so it'll be freed along with that entry and make the space available one entry sooner, increasing the chances for keeping the current entry around longer. If ring buffer is empty then always reset it up front to get best alignment right away i.e. beginning of buffer. Signed-off-by: Nicolas Pitre <[email protected]>
1 parent 082ca85 commit f7c4fe6

File tree

1 file changed

+21
-12
lines changed

1 file changed

+21
-12
lines changed

subsys/shell/shell_history.c

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
* | | header | "history item" | |
3434
* | | no padding | | |
3535
* +-----------------+------------+----------------+--------------------------+
36+
*
37+
* As an optimization, the added padding is attributed to the preceding item
38+
* instead of the current item. This way the padding will be freed one item
39+
* sooner.
3640
*/
3741
struct shell_history_item {
3842
sys_dnode_t dnode;
@@ -161,6 +165,17 @@ void z_shell_history_put(struct shell_history *history, uint8_t *line,
161165
}
162166

163167
do {
168+
if (ring_buf_is_empty(history->ring_buf)) {
169+
/* if history is empty reset ring buffer. Even when
170+
* ring buffer is empty, it is possible that available
171+
* continues memory in worst case equals half of the
172+
* ring buffer capacity. By resetting ring buffer we
173+
* ensure that it is capable to provide continues memory
174+
* of ring buffer capacity length.
175+
*/
176+
ring_buf_reset(history->ring_buf);
177+
}
178+
164179
claim_len = ring_buf_put_claim(history->ring_buf,
165180
(uint8_t **)&h_item, total_len);
166181
/* second allocation may succeed if we were at the end of the
@@ -171,7 +186,11 @@ void z_shell_history_put(struct shell_history *history, uint8_t *line,
171186
ring_buf_put_claim(history->ring_buf,
172187
(uint8_t **)&h_item, total_len);
173188
if (claim2_len == total_len) {
174-
padding += claim_len;
189+
/*
190+
* We may get here only if a previous entry
191+
* exists. Stick the excess padding to it.
192+
*/
193+
h_item->padding += claim_len;
175194
total_len += claim_len;
176195
claim_len = total_len;
177196
}
@@ -184,17 +203,7 @@ void z_shell_history_put(struct shell_history *history, uint8_t *line,
184203
}
185204

186205
ring_buf_put_finish(history->ring_buf, 0);
187-
if (remove_from_tail(history) == false) {
188-
__ASSERT_NO_MSG(ring_buf_is_empty(history->ring_buf));
189-
/* if history is empty reset ring buffer. Even when
190-
* ring buffer is empty, it is possible that available
191-
* continues memory in worst case equals half of the
192-
* ring buffer capacity. By reseting ring buffer we
193-
* ensure that it is capable to provide continues memory
194-
* of ring buffer capacity length.
195-
*/
196-
ring_buf_reset(history->ring_buf);
197-
}
206+
remove_from_tail(history);
198207
} while (1);
199208
}
200209

0 commit comments

Comments
 (0)