public inbox for [email protected]  
help / color / mirror / Atom feed
From: Ronan Dunklau <[email protected]>
To: Tomas Vondra <[email protected]>
To: [email protected]
Cc: Andres Freund <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Cc: David Rowley <[email protected]>
Subject: Re: Use generation context to speed up tuplesorts
Date: Fri, 07 Jan 2022 12:03:55 +0100
Message-ID: <3082578.5fSG56mABF@aivenronan> (raw)
In-Reply-To: <CAApHDvrTNYtHZUjUbYbDJ-hPWNbotnCXr2pN7+EJURirz7urPw@mail.gmail.com>
References: <CAApHDvoH4ASzsAOyHcxkuY01Qf++8JJ0paw+03dk+W25tQEcNQ@mail.gmail.com>
	<[email protected]>
	<CAApHDvrTNYtHZUjUbYbDJ-hPWNbotnCXr2pN7+EJURirz7urPw@mail.gmail.com>

Le vendredi 31 décembre 2021, 22:26:37 CET David Rowley a écrit :
> I've attached some benchmark results that I took recently.  The
> spreadsheet contains results from 3 versions. master, master + 0001 -
> 0002, then master + 0001 - 0003.  The 0003 patch makes the code a bit
> more conservative about the chunk sizes it allocates and also tries to
> allocate the tuple array according to the number of tuples we expect
> to be able to sort in a single batch for when the sort is not
> estimated to fit inside work_mem.

(Sorry for trying to merge back the discussion on the two sides of the thread)

In  https://www.postgresql.org/message-id/4776839.iZASKD2KPV%40aivenronan, I 
expressed the idea of being able to tune glibc's malloc behaviour. 

I implemented that (patch 0001) to provide a new hook which is called on 
backend startup, and anytime we set work_mem. This hook is # defined depending 
on the malloc implementation: currently a default, no-op implementation is 
provided as well as a glibc's malloc implementation.

The glibc's malloc implementation relies on a new GUC, 
glibc_malloc_max_trim_threshold. When set to it's default value of -1, we 
don't tune malloc at all, exactly as in HEAD. If a different value is provided, 
we set M_MMAP_THRESHOLD to half this value, and M_TRIM_TRESHOLD to this value, 
capped by work_mem / 2 and work_mem respectively. 

The net result is that we can then allow to keep more unused memory at the top 
of the heap, and to use mmap less frequently, if the DBA chooses too. A 
possible other use case would be to on the contrary, limit the allocated 
memory in idle backends to a minimum. 

The reasoning behind this is that glibc's malloc default way of handling those 
two thresholds is to adapt to the size of the last freed mmaped block. 

I've run the same "up to 32 columns" benchmark as you did, with this new patch 
applied on top of both HEAD and your v2 patchset incorporating planner 
estimates for the block sizez. Those are called "aset" and "generation" in the 
attached spreadsheet. For each, I've run it with 
glibc_malloc_max_trim_threshold set to -1, 1MB, 4MB and 64MB. In each case 
I've measured two things:
 - query latency, as reported by pgbench
 - total memory allocated by malloc at backend ext after running each query 
three times. This represents the "idle" memory consumption, and thus what we 
waste in malloc inside of releasing back to the system. This measurement has 
been performed using the very small module presented in patch 0002. Please 
note that I in no way propose that we include this module, it was just a 
convenient way for me to measure memory footprint.

My conclusion is that the impressive gains you see from using the generation 
context with bigger blocks mostly comes from the fact that we allocate bigger 
blocks, and that this moves the mmap thresholds accordingly. I wonder how much 
of a difference it would make on other malloc implementation: I'm afraid the 
optimisation presented here would in fact be specific to glibc's malloc, since 
we have almost the same gains with both allocators when tuning malloc to keep 
more memory. I still think both approaches are useful, and would be necessary. 

Since this affects all memory allocations, I need to come up with other 
meaningful scenarios to benchmarks.


-- 
Ronan Dunklau

Attachments:

  [application/vnd.oasis.opendocument.spreadsheet] bench_trims.ods (151.1K, ../3082578.5fSG56mABF@aivenronan/2-bench_trims.ods)
  download

  [text/x-patch] v1-0001-Add-the-possibility-of-tuning-malloc-options.patch (9.4K, ../3082578.5fSG56mABF@aivenronan/3-v1-0001-Add-the-possibility-of-tuning-malloc-options.patch)
  download | inline diff:
From 7cac8c70d916a63d54fbfbb7d6bb8e9c753e71a1 Mon Sep 17 00:00:00 2001
From: Ronan Dunklau <[email protected]>
Date: Mon, 20 Dec 2021 13:29:48 +0100
Subject: [PATCH v1 1/2] Add the possibility of tuning malloc options.

We provide a malloc implementation dependent version of a
MallocAdjustSettings function, which can be used to tune different
malloc options.

As for now, only a version for glibc's malloc is provided. This
implementation tunes M_MMAP_THRESHOLD and M_TRIM_THRESHOLD according to
the work_mem value and a new GUC, glibc_malloc_max_trim_threshold.

If set to -1 (the default) this new tuning is disabled. If set to
something else, we set M_TRIM_THRESHOLD to that value (capped to
work_mem) and M_MMAP_THRESHOLD to half that value (capped to work_mem or
32MB).
---
 src/backend/utils/init/postinit.c      |   9 +++
 src/backend/utils/misc/guc.c           |  37 ++++++++-
 src/backend/utils/mmgr/Makefile        |   1 +
 src/backend/utils/mmgr/malloc_tuning.c | 106 +++++++++++++++++++++++++
 src/include/utils/memutils.h           |  10 +++
 5 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100644 src/backend/utils/mmgr/malloc_tuning.c

diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 7292e51f7d..ca170c430e 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -661,6 +661,12 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
 	InitCatalogCache();
 	InitPlanCache();
 
+	/* Adjust malloc options if needed.
+	 * This is done here because the implementation can vary depending on the
+	 * type of backend.
+	 */
+	MallocAdjustSettings();
+
 	/* Initialize portal manager */
 	EnablePortalManager();
 
@@ -1054,6 +1060,9 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
 	/* Initialize this backend's session state. */
 	InitializeSession();
 
+	/* Tune malloc options according to what we've read */
+	/* MallocTuneHook(); */
+
 	/* report this backend in the PgBackendStatus array */
 	if (!bootstrap)
 		pgstat_bestart();
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f9504d3aec..7c0e124529 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -235,6 +235,11 @@ static bool check_recovery_target_lsn(char **newval, void **extra, GucSource sou
 static void assign_recovery_target_lsn(const char *newval, void *extra);
 static bool check_primary_slot_name(char **newval, void **extra, GucSource source);
 static bool check_default_with_oids(bool *newval, void **extra, GucSource source);
+static void assign_work_mem(int newval, void* extra);
+static void assign_glibc_trim_threshold(int newval, void* extra);
+
+static void check_reserved_prefixes(const char *varName);
+static List *reserved_class_prefix = NIL;
 
 /* Private functions in guc-file.l that need to be called from guc.c */
 static ConfigVariable *ProcessConfigFileInternal(GucContext context,
@@ -2439,7 +2444,7 @@ static struct config_int ConfigureNamesInt[] =
 		},
 		&work_mem,
 		4096, 64, MAX_KILOBYTES,
-		NULL, NULL, NULL
+		NULL, &assign_work_mem, NULL
 	},
 
 	{
@@ -2465,6 +2470,21 @@ static struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"glibc_malloc_max_trim_threshold", PGC_USERSET, RESOURCES_MEM,
+			gettext_noop("Sets the maximum value for glibc's M_TRIM_THRESHOLD option."),
+			gettext_noop("This controls how much memory glibc's will not return to the "
+						 "OS once freed. An idle backend can thus consume that much memory "
+						 "even if not in used. The default (-1) value disable static tuning "
+						 "and relies on the default dynamic adjustment"),
+			GUC_UNIT_KB
+		},
+		&glibc_malloc_max_trim_threshold,
+		-1, -1, MAX_KILOBYTES,
+		NULL, &assign_glibc_trim_threshold, NULL
+	},
+
+
 	/*
 	 * We use the hopefully-safely-small value of 100kB as the compiled-in
 	 * default for max_stack_depth.  InitializeGUCOptions will increase it if
@@ -12570,6 +12590,7 @@ check_primary_slot_name(char **newval, void **extra, GucSource source)
 	return true;
 }
 
+
 static bool
 check_default_with_oids(bool *newval, void **extra, GucSource source)
 {
@@ -12585,4 +12606,18 @@ check_default_with_oids(bool *newval, void **extra, GucSource source)
 	return true;
 }
 
+static void
+assign_work_mem(int newval, void* extra)
+{
+	work_mem = newval;
+	MallocAdjustSettings();
+}
+
+static void
+assign_glibc_trim_threshold(int newval, void* extra)
+{
+	glibc_malloc_max_trim_threshold = newval;
+	MallocAdjustSettings();
+}
+
 #include "guc-file.c"
diff --git a/src/backend/utils/mmgr/Makefile b/src/backend/utils/mmgr/Makefile
index 3b4cfdbd52..ab75a71249 100644
--- a/src/backend/utils/mmgr/Makefile
+++ b/src/backend/utils/mmgr/Makefile
@@ -17,6 +17,7 @@ OBJS = \
 	dsa.o \
 	freepage.o \
 	generation.o \
+	malloc_tuning.o \
 	mcxt.o \
 	memdebug.o \
 	portalmem.o \
diff --git a/src/backend/utils/mmgr/malloc_tuning.c b/src/backend/utils/mmgr/malloc_tuning.c
new file mode 100644
index 0000000000..90526d8cc1
--- /dev/null
+++ b/src/backend/utils/mmgr/malloc_tuning.c
@@ -0,0 +1,106 @@
+#include "postgres.h"
+
+#include "utils/memutils.h"
+#include "miscadmin.h"
+
+
+/*
+ * Implementation speficic GUCs. Those are defined even if we use another implementation, but will have
+ * no effect in that case.
+ */
+
+int glibc_malloc_max_trim_threshold;
+
+/*
+ * Depending on the malloc implementation used, we may want to
+ * tune it.
+ * In this first version, the only tunable library is glibc's malloc
+ * implementation.
+ */
+/* GLIBC implementation */
+#if defined(__GLIBC__)
+#include <malloc.h>
+#include <limits.h>
+#include <bits/wordsize.h>
+
+int previous_mmap_threshold = -1;
+int previous_trim_threshold = -1;
+
+/* For GLIBC's malloc, we want to avoid having too many mmap'd memory regions,
+ * and also to avoid repeatingly allocating / releasing memory from the system.
+ *
+ * The default configuration of malloc adapt's its M_MMAP_THRESHOLD and M_TRIM_THRESHOLD
+ * dynamically when previoulsy mmaped blocks are freed.
+ *
+ * This isn't really sufficient for our use case, as we may end up with a trim
+ * threshold which repeatedly releases work_mem memory to the system.
+ *
+ * Instead of letting malloc dynamically tune itself, for values up to 32MB we
+ * ensure that work_mem will fit both bellow M_MMAP_THRESHOLD and
+ * M_TRIM_THRESHOLD. The rationale for this is that once a backend has allocated
+ * this much memory, it is likely to use it again.
+ *
+ * To keep up with malloc's default behaviour, we set M_TRIM_THRESHOLD to
+ * M_MMAP_THRESHOLD * 2 so that work_mem blocks can avoid being released too
+ * early.
+ *
+ * Newer versions of malloc got rid of the MALLOC_MMAP_THRESHOLD upper limit,
+ * but we still enforce the values it sets to avoid wasting too much memory if we have a huge
+ * work_mem which is used only once.
+ */
+
+# if __WORDSIZE == 32
+#  define MMAP_THRESHOLD_MAX (512 * 1024)
+# else
+#  define MMAP_THRESHOLD_MAX (4 * 1024 * 1024 * sizeof(long))
+# endif
+
+void
+MallocAdjustSettings()
+{
+	int uncapped_mmap_threshold,
+		mmap_threshold,
+		trim_threshold;
+	long base_target;
+	/* If static malloc tuning is disabled, bail out. */
+	if (glibc_malloc_max_trim_threshold == -1)
+		return;
+	/* We don't want to adjust anything in the postmaster process, as that would
+	 * disable dynamic adjustment for any child process*/
+	if ((MyProcPid == PostmasterPid) ||
+		((MyBackendType != B_BACKEND) &&
+		 (MyBackendType != B_BG_WORKER)))
+		return;
+	base_target = Min((long) work_mem * 1024, (long) glibc_malloc_max_trim_threshold / 2 * 1024);
+	/* To account for overhead, add one more memory page to that. */
+	base_target += 4096;
+	uncapped_mmap_threshold = Min(INT_MAX, base_target);
+	/* Cap mmap_threshold to MMAP_THRESHOLD_MAX */
+	mmap_threshold = Min(MMAP_THRESHOLD_MAX, uncapped_mmap_threshold);
+	/* Set trim treshold to two times the uncapped value */
+	trim_threshold = Min(INT_MAX, (long) uncapped_mmap_threshold * 2);
+	if (mmap_threshold != previous_mmap_threshold)
+	{
+		mallopt(M_MMAP_THRESHOLD, mmap_threshold);
+		previous_mmap_threshold = mmap_threshold;
+	}
+
+	if (trim_threshold != previous_trim_threshold)
+	{
+		mallopt(M_TRIM_THRESHOLD, trim_threshold);
+		/* If we reduce the trim_threshold, ask malloc to actually trim it.
+		 * This allows us to recover from a bigger work_mem set up once, then
+		 * reset back to a smaller value.
+		 */
+		if (trim_threshold < previous_trim_threshold)
+			malloc_trim(trim_threshold);
+		previous_trim_threshold = trim_threshold;
+	}
+}
+
+/* Default no-op implementation for others malloc providers. */
+#else
+void MallocAdjustSettings()
+{
+}
+#endif
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index ff872274d4..b958926969 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -89,6 +89,16 @@ extern void MemoryContextStatsDetail(MemoryContext context, int max_children,
 extern void MemoryContextAllowInCriticalSection(MemoryContext context,
 												bool allow);
 
+/* Interface to tune underlying malloc implementation in mcxt.c.
+ * The implementation can only rely on GUCs for now, but it could be profitable
+ * to collect statistics about individual palloc / pfree cycle to determine the
+ * optimum size of certain values.
+ */
+extern void MallocAdjustSettings(void);
+
+/* Malloc-implementation specific GUCs */
+extern PGDLLIMPORT int glibc_malloc_max_trim_threshold;
+
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void MemoryContextCheck(MemoryContext context);
 #endif
-- 
2.34.1



  [text/x-patch] v1-0002-Add-malloc_stats-extension-to-see-the-allocated-m.patch (2.0K, ../3082578.5fSG56mABF@aivenronan/4-v1-0002-Add-malloc_stats-extension-to-see-the-allocated-m.patch)
  download | inline diff:
From 7ba82ff640f7151d880a9d49666ea5c260a6d3b6 Mon Sep 17 00:00:00 2001
From: Ronan Dunklau <[email protected]>
Date: Wed, 22 Dec 2021 12:07:57 +0100
Subject: [PATCH v1 2/2] Add malloc_stats extension to see the allocated memory
 at the end

---
 contrib/malloc_stats/Makefile       | 16 ++++++++++++
 contrib/malloc_stats/malloc_stats.c | 39 +++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 contrib/malloc_stats/Makefile
 create mode 100644 contrib/malloc_stats/malloc_stats.c

diff --git a/contrib/malloc_stats/Makefile b/contrib/malloc_stats/Makefile
new file mode 100644
index 0000000000..b92063f5f5
--- /dev/null
+++ b/contrib/malloc_stats/Makefile
@@ -0,0 +1,16 @@
+# contrib/malloc_stats/Makefile
+
+MODULES = malloc_stats
+
+PGFILEDESC = "malloc_stats - register on_proc_exit to dump malloc_stats"
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/malloc_stats
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/malloc_stats/malloc_stats.c b/contrib/malloc_stats/malloc_stats.c
new file mode 100644
index 0000000000..802100b854
--- /dev/null
+++ b/contrib/malloc_stats/malloc_stats.c
@@ -0,0 +1,39 @@
+#include "postgres.h"
+#include "storage/ipc.h"
+#include "fmgr.h"
+#include "utils/builtins.h"
+
+#include "malloc.h"
+
+PG_MODULE_MAGIC;
+
+void _PG_init(void);
+void malloc_exit_callback(int, Datum);
+
+bool proc_exit_setup = false;
+
+void
+malloc_exit_callback(int code, Datum arg)
+{
+	struct mallinfo2 m;
+	m = mallinfo2();
+	elog(NOTICE, "On exit malloc stats: \n"
+		 		 "\tTotal memory: %ld\n"
+				 "\tMemory (without mmap): %ld\n"
+				 "\tMmaped memory: %ld\n"
+				 "\tIn-use: %ld\n"
+				 "\tFree blocks: %ld\n"
+				 "\tKeep cost: %ld",
+			 m.arena + m.hblkhd,
+			 m.arena,
+			 m.hblkhd,
+			 m.uordblks,
+			 m.fordblks,
+			 m.keepcost);
+}
+
+void
+_PG_init(void)
+{
+	on_proc_exit(malloc_exit_callback, 0);
+}
-- 
2.34.1



view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Use generation context to speed up tuplesorts
  In-Reply-To: <3082578.5fSG56mABF@aivenronan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox