Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] feedback appreciated
@ 2019-10-18 22:58 Alex Diaz
  0 siblings, 0 replies; only message in thread
From: Alex Diaz @ 2019-10-18 22:58 UTC (permalink / raw)
  To: kernelnewbies

[-- Attachment #1: Type: text/html, Size: 4231 bytes --]

<div dir='auto'>Hi<div dir="auto"><br></div><div dir="auto">I sent a patch in and I got ignored. I'm thinking I did something wrong and I would like some feedback.&nbsp;</div><div dir="auto"><br></div><div dir="auto">There's more backstory in my original email so here it is:</div><div dir="auto"><br></div><div dir="auto" class="elided-text">---------- Forwarded message ----------<br>From: Alex &lt;alex@akdev.xyz&gt;<br>Date: Sep. 22, 2019 12:50<br>Subject: [PATCH] perf: save space when filteting events]<br>To: alex@akdev.xyz<br>Cc: <br><br type="attribution"><blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex"><div>I was browsing the code in perf and I found a `FIXME` comment in the code
<br>that builds the binary expressions to filter events/syscalls:r
<br>`perf trace -e write,read`
<br>
<br>The code mentioned wanting to use an implementation of log10() in the dvb
<br>drivers. I am not aware why that specific implementation is mentioned.
<br>I used the implementation found in `math.h`.
<br>
<br>I tested this patch passing 200 syscalls to filter on a `perf trace -e ${syscalls} ls`
<br>call. You can see the commands used and a snipped of the vallgrind results here:
<br><a href="https://termbin.com/k4of">https://termbin.com/k4of</a>
<br>
<br>Before: 61,910,110 bytes allocated
<br>After:  61,907,045 bytes allocated
<br>
---</div></blockquote></div><div dir="auto" class="elided-text"><blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex"><div>perf used to allocate space in excess to build the filtering expressions.
<br>now perf only allocates the space necessary and not more.
<br>
<br>Signed-off-by: Alex Diaz &lt;<a href="mailto:alex@akdev.xyz">alex@akdev.xyz</a>&gt;
<br>---
<br> tools/perf/util/string.c  | 25 ++++++++++++++++++++-----
<br> tools/perf/util/string2.h |  2 ++
<br> 2 files changed, 22 insertions(+), 5 deletions(-)
<br>
<br>diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
<br>index 52603876c548..4c3913a9e7e7 100644
<br>--- a/tools/perf/util/string.c
<br>+++ b/tools/perf/util/string.c
<br>@@ -3,6 +3,7 @@
<br> #include 
<br> #include 
<br> #include 
<br>+#include 
<br>
<br> #include 
<br>
<br>@@ -209,15 +210,29 @@ int strtailcmp(const char *s1, const char *s2)
<br>         return 0;
<br> }
<br>
<br>-char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints)
<br>+size_t calc_expr_buffer_size(const char *var, size_t nints, int *ints)
<br> {
<br>         /*
<br>-        * FIXME: replace this with an expression using log10() when we
<br>-        * find a suitable implementation, maybe the one in the dvb drivers...
<br>+        * Calculate the buffer for the expression:
<br>          *
<br>-        * "%s == %d || " = log10(MAXINT) * 2 + 8 chars for the operators
<br>+        * "%s == %d || "
<br>+        * length: strlen(var) + strlen(" == ") + log10(n) + strlen(" || ") + 1
<br>          */
<br>-       size_t size = nints * 28 + 1; /* \0 */
<br>+       size_t size = 0;
<br>+       size_t num_len = 0;
<br>+       const size_t var_len = strlen(var);
<br>+
<br>+       for (size_t i = 0; i &lt; nints; ++i) {
<br>+               num_len = (ints[i] == 0) ? 1 : log10(ints[i]);
<br>+               size += var_len + num_len + 9;
<br>+       }
<br>+
<br>+       return size;
<br>+}
<br>+
<br>+char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints)
<br>+{
<br>+       size_t size = calc_expr_buffer_size(var, nints, ints);
<br>         size_t i, printed = 0;
<br>         char *expr = malloc(size);
<br>
<br>diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
<br>index 708805f5573e..28fbc782ad54 100644
<br>--- a/tools/perf/util/string2.h
<br>+++ b/tools/perf/util/string2.h
<br>@@ -20,6 +20,8 @@ static inline bool strisglob(const char *str)
<br> }
<br> int strtailcmp(const char *s1, const char *s2);
<br>
<br>+size_t calc_expr_buffer_size(const char *var, size_t nints, int *ints);
<br>+
<br> char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints);
<br>
<br> static inline char *asprintf_expr_in_ints(const char *var, size_t nints, int *ints)
<br>--
<br>2.21.0
<br>
<br></div></blockquote></div><br><br></div>



[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, back to index

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 22:58 [PATCH] feedback appreciated Alex Diaz

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org
	public-inbox-index kernelnewbies

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git