git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] CI: add SANITIZE=[address|undefined] jobs

From
Jeff King <peff@peff.net>
Date
Jul 28, 2022, 21:31 UTC
Message-ID
<YuMAIfM3jv6lI9SA@coredump.intra.peff.net>
In-Reply-To
<YuL7EotrIpnOn5BT@coredump.intra.peff.net>
On Thu, Jul 28, 2022 at 05:09:38PM -0400, Jeff King wrote:
Show 7 quoted lines
> > Ditto TEST_NO_MALLOC_CHECK=1 & --no-bin-wrappers, but we can think about
> > all of those some other time....
> 
> I'd be surprised if the malloc checking itself is all that expensive,
> though it does look like we call getconf and expr once per test there
> for setup. We could almost certainly hoist that out and call it once per
> script.

Nope, I was totally wrong here. Running with TEST_NO_MALLOC_CHECK=1 does indeed make a big difference:

  Benchmark 1: make test
    Time (mean ± σ):     67.486 s ±  3.339 s    [User: 622.643 s, System: 409.951 s]
    Range (min … max):   63.634 s … 69.550 s    3 runs
   
  Benchmark 2: TEST_NO_MALLOC_CHECK=1 make test
    Time (mean ± σ):     57.596 s ±  0.899 s    [User: 577.273 s, System: 291.627 s]
    Range (min … max):   57.072 s … 58.634 s    3 runs

Eliminating the extra per-test processes with the patch below helps a little, but most of the effort really is (presumably) going to the actual malloc checking code:

  Benchmark 1: make test [nb: after applying patch...]
  Time (mean ± σ):     67.091 s ±  0.385 s    [User: 599.382 s, System: 410.781 s]
  Range (min … max):   66.648 s … 67.343 s    3 runs

Curious that most of the CPU time seems to go into system time. I'd have thought it was extra internal malloc debugging bits, but maybe it is allocating mmap/brk calls in a less efficient way. I dunno how any of it actually works.

-Peff
---
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 55857af601..303fbe8ecc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -557,14 +557,20 @@ then
 		: nothing
 	}
 else
+	if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) &&
+	   _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} &&
+	    expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null
+	then
+		USE_LIBC_MALLOC_DEBUG=t
+	else
+		USE_LIBC_MALLOC_DEBUG=
+	fi
 	setup_malloc_check () {
 		local g
 		local t
 		MALLOC_CHECK_=3	MALLOC_PERTURB_=165
 		export MALLOC_CHECK_ MALLOC_PERTURB_
-		if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) &&
-		   _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} &&
-		   expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null
+		if test -n "$USE_LIBC_MALLOC_DEBUG"
 		then
 			g=
 			LD_PRELOAD="libc_malloc_debug.so.0"
Previous: Jeff KingNext: Ævar Arnfjörð Bjarmason
Message 11 of 12 in “CI: add SANITIZE=[address|undefined] jobs”
  1. CI: add SANITIZE=[address|undefined] jobsÆvar Arnfjörð Bjarmason, Jul 26, 2022
  2. Derrick StoleeJul 26, 2022
  3. Ævar Arnfjörð BjarmasonJul 26, 2022
  4. Derrick StoleeJul 27, 2022
  5. Junio C HamanoJul 27, 2022
  6. Ævar Arnfjörð BjarmasonJul 28, 2022
  7. Junio C HamanoJul 28, 2022
  8. Jeff KingJul 27, 2022
  9. Ævar Arnfjörð BjarmasonJul 28, 2022
  10. Jeff KingJul 28, 2022
  11. Jeff KingJul 28, 2022
  12. Ævar Arnfjörð BjarmasonJul 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.