Re: Large writable variables
Andres Freund <andres@anarazel.de>
Hi,
After the last few changes (including a proposed one), we now have in
the .data segment (i.e. mutable initialized variables):
$ objdump -j .data -t ~/build/postgres/dev-optimize/vpath/src/backend/postgres|awk '{print $4, $5, $6}'|sort -r|lessGreetings,
.data 0000000000004380 ConfigureNamesInt
.data 0000000000003570 ConfigureNamesBool
.data 0000000000002140 ConfigureNamesString
.data 00000000000010e0 ConfigureNamesEnum
.data 0000000000000d20 ConfigureNamesReal
These are by far the largest chunk of the .data segment (like 47152
bytes out of 51168 bytes). It's not entirely trivial to fix. While we
can slap consts onto large parts of the functions taking a struct
config_generic*, there's a few places where *do* modify them.
ISTM, to actually fix, we'd have to split
struct config_generic
{
/* constant fields, must be set correctly in initial value: */
const char *name; /* name of variable - MUST BE FIRST */
GucContext context; /* context required to set the variable */
enum config_group group; /* to help organize variables by function */
const char *short_desc; /* short desc. of this variable's purpose */
const char *long_desc; /* long desc. of this variable's purpose */
int flags; /* flag bits, see guc.h */
/* variable fields, initialized at runtime: */
enum config_type vartype; /* type of variable (set only at startup) */
int status; /* status bits, see below */
GucSource source; /* source of the current actual value */
GucSource reset_source; /* source of the reset_value */
GucContext scontext; /* context that set the current value */
GucContext reset_scontext; /* context that set the reset value */
GucStack *stack; /* stacked prior values */
void *extra; /* "extra" pointer for current actual value */
char *sourcefile; /* file current setting is from (NULL if not
* set in config file) */
int sourceline; /* line in source file */
};
into two different arrays. Namely the already denoted 'constant fields'
and 'variable fields. But it's a bit more complicated than that: The
size doesn't come just from the base config_struct, but also from the
config_{bool,int,real,string,enum} arrays. Where we again have separate
'constant fields' and 'variable fields'.
So we'd have to refactor more heavily than just splitting the above
array into two.
I kind of wonder, if we shouldn't remove the separate ConfigureNamesInt*
int arrays and change things so we have one
struct config_def
{
const char *name; /* name of variable - MUST BE FIRST */
GucContext context; /* context required to set the variable */
enum config_group group; /* to help organize variables by function */
const char *short_desc; /* short desc. of this variable's purpose */
const char *long_desc; /* long desc. of this variable's purpose */
int flags; /* flag bits, see guc.h */
enum config_type vartype; /* type of variable (set only at startup) */
union
{
struct
{
bool *variable;
bool boot_val;
GucBoolCheckHook check_hook;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
} config_bool;
struct
{
int *variable;
int boot_val;
int min;
int max;
GucIntCheckHook check_hook;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
} config_int
...
} pertype;
}
and then a corresponding struct config_val with a similar union.
Besides reducing modified memory, it'd also have the benefit of making
the grouping within the various arrays much more sensible, because
related fields could be grouped together.
.data 0000000000000420 intRelOpts
.data 00000000000001c0 realRelOpts
.data 0000000000000118 boolRelOpts
.data 00000000000000a8 stringRelOpts
these should be readonly, but the code doesn't make that easy. There's
pending refactorings, I don't know how they effect this.
.data 0000000000000068 SnapshotSelfData
.data 0000000000000068 SnapshotAnyData
.data 0000000000000068 SecondarySnapshotData
.data 0000000000000068 CurrentSnapshotData
.data 0000000000000068 CatalogSnapshotData
The obviously actually are modifyable.
.data 0000000000000028 spi_printtupDR
.data 0000000000000028 printsimpleDR
.data 0000000000000028 donothingDR
.data 0000000000000028 debugtupDR
These we could actually make constant, but CreateDestReceiver() as an
API makes that inconvenient. They also are pretty darn small... There's
a security benefit in making them constant and casting the constness
away - I think that might not be insane.
- Andres
Commits
-
Apply unconstify() in more places
- 0a8590b2a09e 12.0 landed
-
Improve unconstify() documentation
- f2898de98a54 12.0 landed
-
Drop const cast from dlsym() calls
- e6f5d1accd3a 12.0 landed
-
Const-ify a few more large static tables.
- 48d818ede15b 12.0 landed
-
Improve tzparse's handling of TZDEFRULES ("posixrules") zone data.
- fb38d9a2e1ed 9.5.15 landed
- ec5fe7f799a4 9.4.20 landed
- e7eb07f70951 12.0 landed
- db4f9c0258be 11.1 landed
- c776cd47245a 9.3.25 landed
- 312f632005f3 10.6 landed
- 1164389e12b0 9.6.11 landed
-
Avoid statically allocating statement cache in ecpglib/prepare.c.
- e15aae829e3e 12.0 landed
-
Reorder FmgrBuiltin members, saving 25% in size.
- 28d750c0cd5b 12.0 landed
-
Add macro to cast away const without allowing changes to underlying type.
- d1211c63f010 12.0 landed
-
Mark constantly allocated dest receiver as const.
- 93ca02e00560 12.0 landed
-
Avoid statically allocating formatting.c's format string caches.
- fd85e9f78d44 12.0 landed
-
Correct constness of system attributes in heap.c & prerequisites.
- 02a30a09f9e5 12.0 landed
-
Avoid statically allocating gmtsub()'s timezone workspace.
- baa94ebe2cc8 9.3.25 landed
- 5ef71d4bd322 9.6.11 landed
- 27ba589b745f 9.4.20 landed
- 18c725413fad 9.5.15 landed
- d64a54fb9c81 10.6 landed
- d11268237378 11.1 landed
- 3dfef0c519de 12.0 landed
-
Correct constness of a few variables.
- 62649bad831f 12.0 landed
-
Move the replication lag tracker into heap memory.
- e73ca79fc78d 12.0 landed