From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Mon, 8 Aug 2022 16:40:15 +0000 (+0200)
Subject: lib/generic/array: avoid quadratic work for long arrays
X-Git-Tag: v5.5.2~2^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=82867bbda809600313580c407917e7907ca6abab;p=thirdparty%2Fknot-resolver.git

lib/generic/array: avoid quadratic work for long arrays

For long arrays we really want to increase their length by a fraction.
Otherwise it will cost lots of CPU.  Doubling seems customary,
though I could imagine e.g. keeping the +50% growth on longest arrays.

I finally got sufficiently angry with this piece of code when debugging
https://forum.turris.cz/t/how-to-debug-a-custom-hosts-file-for-kresd/17449
though in that case it wasn't the main source of inefficiency.

CI: two of the mysterious/bogus warnings around arrays disappeared.
---

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 11f360e0f..baa09aca7 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -243,7 +243,7 @@ lint:scan-build:
   script:
     - export SCANBUILD="$(realpath ./scripts/run-scanbuild-with-args.sh)"
     - ninja -C build_ci* scan-build || true
-    - test "$(ls build_ci*/meson-logs/scanbuild/*/report-*.html | wc -l)" = 24 # we have this many errors ATM :-)
+    - test "$(ls build_ci*/meson-logs/scanbuild/*/report-*.html | wc -l)" = 22 # we have this many errors ATM :-)
 
 lint:tidy:
   <<: *after_build
diff --git a/lib/generic/array.h b/lib/generic/array.h
index b20a802a6..6f969a222 100644
--- a/lib/generic/array.h
+++ b/lib/generic/array.h
@@ -53,14 +53,17 @@
 #pragma once
 #include <stdlib.h>
 
-/** Simplified Qt containers growth strategy. */
-static inline size_t array_next_count(size_t want)
+/** Choose array length when it overflows. */
+static inline size_t array_next_count(size_t elm_size, size_t want, size_t have)
 {
-	if (want < 2048) {
-		return (want < 20) ? want + 4 : want * 2;
-	} else {
-		return want + 2048;
-	}
+	if (want >= have * 2) // We amortized enough and maybe more won't be needed.
+		return want;
+	const size_t want_b = want * elm_size;
+	if (want_b < 64) // Short arrays are cheap to copy; get just one extra.
+		return want + 1;
+	if (want_b < 1024) // 50% growth amortizes to roughly 3 copies per element.
+		return want + want / 2;
+	return want * 2; // Doubling growth amortizes to roughly 2 copies per element.
 }
 
 /** @internal Incremental memory reservation */
@@ -70,7 +73,7 @@ static inline int array_std_reserve(void *baton, void **mem, size_t elm_size, si
 		return 0;
 	}
 	/* Simplified Qt containers growth strategy */
-	size_t next_size = array_next_count(want);
+	size_t next_size = array_next_count(elm_size, want, *have);
 	void *mem_new = realloc(*mem, next_size * elm_size);
 	if (mem_new != NULL) {
 		*mem = mem_new;
diff --git a/lib/utils.c b/lib/utils.c
index 46d3fb65a..da2b8236c 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -185,7 +185,7 @@ int kr_memreserve(void *baton, void **mem, size_t elm_size, size_t want, size_t
         return 0;
     } else {
         knot_mm_t *pool = baton;
-        size_t next_size = array_next_count(want);
+        size_t next_size = array_next_count(elm_size, want, *have);
         void *mem_new = mm_alloc(pool, next_size * elm_size);
         if (mem_new != NULL) {
 	    if (*mem) { /* 0-length memcpy from NULL isn't technically OK */