From f7c4fe677823ae47ce8aa87af8514ef4100a6172 Mon Sep 17 00:00:00 2001
From: Nicolas Pitre <npitre@baylibre.com>
Date: Wed, 16 Feb 2022 17:10:40 -0500
Subject: [PATCH] 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 <npitre@baylibre.com>
---
 subsys/shell/shell_history.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/subsys/shell/shell_history.c b/subsys/shell/shell_history.c
index b3ef5f917a2..688525a93cb 100644
--- a/subsys/shell/shell_history.c
+++ b/subsys/shell/shell_history.c
@@ -33,6 +33,10 @@
  * |                 | header     | "history item" |                          |
  * |                 | no padding |                |                          |
  * +-----------------+------------+----------------+--------------------------+
+ *
+ * As an optimization, the added padding is attributed to the preceding item
+ * instead of the current item. This way the padding will be freed one item
+ * sooner.
  */
 struct shell_history_item {
 	sys_dnode_t dnode;
@@ -161,6 +165,17 @@ void z_shell_history_put(struct shell_history *history, uint8_t *line,
 	}
 
 	do {
+		if (ring_buf_is_empty(history->ring_buf)) {
+			/* if history is empty reset ring buffer. Even when
+			 * ring buffer is empty, it is possible that available
+			 * continues memory in worst case equals half of the
+			 * ring buffer capacity. By resetting ring buffer we
+			 * ensure that it is capable to provide continues memory
+			 * of ring buffer capacity length.
+			 */
+			ring_buf_reset(history->ring_buf);
+		}
+
 		claim_len = ring_buf_put_claim(history->ring_buf,
 						(uint8_t **)&h_item, total_len);
 		/* 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,
 				ring_buf_put_claim(history->ring_buf,
 						   (uint8_t **)&h_item, total_len);
 			if (claim2_len == total_len) {
-				padding += claim_len;
+				/*
+				 * We may get here only if a previous entry
+				 * exists. Stick the excess padding to it.
+				 */
+				h_item->padding += claim_len;
 				total_len += claim_len;
 				claim_len = total_len;
 			}
@@ -184,17 +203,7 @@ void z_shell_history_put(struct shell_history *history, uint8_t *line,
 		}
 
 		ring_buf_put_finish(history->ring_buf, 0);
-		if (remove_from_tail(history) == false) {
-			__ASSERT_NO_MSG(ring_buf_is_empty(history->ring_buf));
-			/* if history is empty reset ring buffer. Even when
-			 * ring buffer is empty, it is possible that available
-			 * continues memory in worst case equals half of the
-			 * ring buffer capacity. By reseting ring buffer we
-			 * ensure that it is capable to provide continues memory
-			 * of ring buffer capacity length.
-			 */
-			ring_buf_reset(history->ring_buf);
-		}
+		remove_from_tail(history);
 	} while (1);
 }