-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys: net: gnrc: only reallocate if necessary #6998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Tested it with various packet formats and sizes in gnrc_networking on samr21-xpro and the following patch to be able to check for memory leaks: diff --git a/examples/gnrc_networking/main.c b/examples/gnrc_networking/main.c
index 6301f42..52637ca 100644
--- a/examples/gnrc_networking/main.c
+++ b/examples/gnrc_networking/main.c
@@ -22,14 +22,24 @@
#include "shell.h"
#include "msg.h"
+#include "net/gnrc/pktbuf.h"
#define MAIN_QUEUE_SIZE (8)
static msg_t _main_msg_queue[MAIN_QUEUE_SIZE];
extern int udp_cmd(int argc, char **argv);
+static int pktbuf_cmd(int argc, char **argv)
+{
+ (void)argc;
+ (void)argv;
+ gnrc_pktbuf_stats();
+ return 0;
+}
+
static const shell_command_t shell_commands[] = {
{ "udp", "send data over UDP and listen on UDP ports", udp_cmd },
+ { "pktbuf", "---", pktbuf_cmd },
{ NULL, NULL, NULL }
};
All seems well. |
I don't see the 'big benefit' behind this but I can confirm it improves performance by about 1-1,5% (evaluated with my l2 reflector). I tested with a UDP application sending packets with payloads: |
I don't see the benefit of the How large can the error (size diff) be for Otherwise one could argue, that we could save the first read to get |
@miri64 do you personally think this change is useful? It mainly changes your own code. |
@PeterKietzmann wasn't this based on your recommendation, that realloc is causing timing problems (at least from what I remember). As such, I think this is useful, yes. |
Ping @PeterKietzmann? |
This did not fix the actual performance regression back then. But at least it improved processing at about ~1% or so. Anyway, both @smlng and me seem to be unconfined about the usefulness of this PR. That's why I'm asking for your personal opinion, since you wrote the initial code. I won't NACK this PR. So in case you are in favor of this PR, please rebase. |
The code was originally by @kaspar030, but I think it is useful, as it frees wasted packet buffer space as soon as possible. As to if we could just remove the I would need to redo this PR for |
Reinstating #6294.