From 7493f5a1ae6ed0e1315680ab707f86231e1691cb Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=A9dric=20Villemain?= Date: Sat, 18 Jun 2016 15:21:06 +0200 Subject: [PATCH] Review code with HAVE_FINCORE also change to not set it by default as it requires a linux patch added tets for the drawer function --- README.rst | 36 ++++++++------ expected/pgfincore.ext.out | 9 ++++ fincore.h | 15 ------ pgfincore.c | 96 ++++++++++++++------------------------ sql/pgfincore.ext.sql | 4 ++ uninstall_pgfincore.sql | 2 +- 6 files changed, 72 insertions(+), 90 deletions(-) delete mode 100644 fincore.h diff --git a/README.rst b/README.rst index ed4a952..b3c180c 100644 --- a/README.rst +++ b/README.rst @@ -84,10 +84,10 @@ Get current state of a relation May be useful:: cedric=# select * from pgfincore('pgbench_accounts'); - relpath | segment | os_page_size | rel_os_pages | pages_mem | group_mem | os_pages_free | databit - --------------------+---------+--------------+--------------+-----------+-----------+---------------+--------- - base/11874/16447 | 0 | 4096 | 262144 | 262144 | 1 | 81016 | - base/11874/16447.1 | 1 | 4096 | 65726 | 65726 | 1 | 81016 | + relpath | segment | os_page_size | rel_os_pages | pages_mem | group_mem | os_pages_free | databit | pages_dirty | group_dirty + --------------------+---------+--------------+--------------+-----------+-----------+---------------+---------+-------------+------------- + base/11874/16447 | 0 | 4096 | 262144 | 262144 | 1 | 81016 | | 0 | 0 + base/11874/16447.1 | 1 | 4096 | 65726 | 65726 | 1 | 81016 | | 0 | 0 (2 rows) Time: 31.563 ms @@ -207,21 +207,24 @@ SYNOPSIS OUT relpath text, OUT segment int, OUT os_page_size bigint, OUT rel_os_pages bigint, OUT pages_mem bigint, OUT group_mem bigint, OUT os_pages_free bigint, - OUT databit varbit) + OUT databit varbit, OUT pages_dirty bigint, + OUT group_dirty bigint) RETURNS setof record pgfincore(IN relname regclass, IN getdatabit bool, OUT relpath text, OUT segment int, OUT os_page_size bigint, OUT rel_os_pages bigint, OUT pages_mem bigint, OUT group_mem bigint, OUT os_pages_free bigint, - OUT databit varbit) + OUT databit varbit, OUT pages_dirty bigint, + OUT group_dirty bigint) RETURNS setof record pgfincore(IN relname regclass, OUT relpath text, OUT segment int, OUT os_page_size bigint, OUT rel_os_pages bigint, OUT pages_mem bigint, OUT group_mem bigint, OUT os_pages_free bigint, - OUT databit varbit) + OUT databit varbit, OUT pages_dirty bigint, + OUT group_dirty bigint) RETURNS setof record DOCUMENTATION @@ -334,10 +337,10 @@ This function provide information about the file system cache (page cache). :: cedric=# select * from pgfincore('pgbench_accounts'); - relpath | segment | os_page_size | rel_os_pages | pages_mem | group_mem | os_pages_free | databit - --------------------+---------+--------------+--------------+-----------+-----------+---------------+--------- - base/11874/16447 | 0 | 4096 | 262144 | 3 | 1 | 408444 | - base/11874/16447.1 | 1 | 4096 | 65726 | 0 | 0 | 408444 | + relpath | segment | os_page_size | rel_os_pages | pages_mem | group_mem | os_pages_free | databit | pages_dirty | group_dirty + --------------------+---------+--------------+--------------+-----------+-----------+---------------+---------+-------------+------------- + base/11874/16447 | 0 | 4096 | 262144 | 3 | 1 | 408444 | | 0 | 0 + base/11874/16447.1 | 1 | 4096 | 65726 | 0 | 0 | 408444 | | 0 | 0 For the specified relation it returns: @@ -351,6 +354,8 @@ For the specified relation it returns: * os_page_free : the number of free page in the OS page cache * databit : the varbit map of the file, because of its size it is useless to output Use pgfincore('pgbench_accounts',true) to activate it. + * pages_dirty : if HAVE_FINCORE constant is define and the platorm provides the relevant information, like pages_mem but for dirtied pages + * group_dirty : if HAVE_FINCORE constant is define and the platorm provides the relevant information, like group_mem but for dirtied pages DEBUG ===== @@ -362,11 +367,14 @@ For example:: set client_min_messages TO debug1; -- debug5 is only usefull to trace each block +REQUIREMENTS +============ + + * PgFincore needs mincore() or fincore() and POSIX_FADVISE + LIMITATIONS =========== - * PgFincore needs mincore() and POSIX_FADVISE. - * PgFincore has a limited mode when POSIX_FADVISE is not provided by the platform. * PgFincore needs PostgreSQL >= 8.3 @@ -378,6 +386,6 @@ SEE ALSO 2ndQuadrant, PostgreSQL Expertise, developement, training and 24x7 support: - http://2ndQuadrant.fr + http://2ndQuadrant.com diff --git a/expected/pgfincore.ext.out b/expected/pgfincore.ext.out index 8191dc0..d7258ec 100644 --- a/expected/pgfincore.ext.out +++ b/expected/pgfincore.ext.out @@ -111,3 +111,12 @@ select true from pgfadvise_normal('test'); t (1 row) +-- +-- tests drawers +-- +select NULL || pgfincore_drawer(databit) from pgfincore('test','main',true); + ?column? +---------- + +(1 row) + diff --git a/fincore.h b/fincore.h deleted file mode 100644 index ddbbb5d..0000000 --- a/fincore.h +++ /dev/null @@ -1,15 +0,0 @@ -//#define _GNU_SOURCE -#include -#include -#include -#include -#define __NR_fincore 316 - -#define HAVE_FINCORE 1 - -int fincore(unsigned int fd, loff_t start, loff_t len, unsigned char * vec); - -int fincore(unsigned int fd, loff_t start, loff_t len, unsigned char * vec) -{ - return syscall(__NR_fincore, fd, start, len, vec); -} diff --git a/pgfincore.c b/pgfincore.c index 118f70e..6edebdc 100644 --- a/pgfincore.c +++ b/pgfincore.c @@ -15,8 +15,6 @@ #include /* fadvise */ /* } */ -#include "fincore.h" - /* { PostgreSQL stuff */ #include "postgres.h" /* general Postgres declarations */ #include "access/heapam.h" /* relation_open */ @@ -52,11 +50,7 @@ PG_MODULE_MAGIC; #define PGSYSCONF_COLS 3 #define PGFADVISE_COLS 4 #define PGFADVISE_LOADER_COLS 5 -#ifndef HAVE_FINCORE -#define PGFINCORE_COLS 8 -#else #define PGFINCORE_COLS 10 -#endif #define PGF_WILLNEED 10 #define PGF_DONTNEED 20 @@ -66,6 +60,7 @@ PG_MODULE_MAGIC; #define FINCORE_PRESENT 0x1 #define FINCORE_DIRTY 0x2 +#define FINCORE_BITS 2 /* * pgfadvise_fctx structure is needed @@ -127,10 +122,8 @@ typedef struct size_t rel_os_pages; size_t pages_mem; size_t group_mem; -#ifdef HAVE_FINCORE size_t pages_dirty; size_t group_dirty; -#endif VarBit *databit; } pgfincoreStruct; @@ -148,11 +141,7 @@ static int pgfadvise_loader_file(char *filename, Datum pgfincore(PG_FUNCTION_ARGS); static int pgfincore_file(char *filename, pgfincoreStruct *pgfncr); - -#ifdef HAVE_FINCORE Datum pgfincore_drawer(PG_FUNCTION_ARGS); -#endif - /* * We need to add some handler to keep the code clean @@ -755,9 +744,7 @@ static int pgfincore_file(char *filename, pgfincoreStruct *pgfncr) { int flag=1; -#ifdef HAVE_FINCORE int flag_dirty=1; -#endif int len, bitlen; bits8 *r; @@ -789,10 +776,8 @@ pgfincore_file(char *filename, pgfincoreStruct *pgfncr) */ pgfncr->pages_mem = 0; pgfncr->group_mem = 0; -#ifdef HAVE_FINCORE pgfncr->pages_dirty = 0; pgfncr->group_dirty = 0; -#endif pgfncr->rel_os_pages = 0; /* @@ -852,31 +837,24 @@ pgfincore_file(char *filename, pgfincoreStruct *pgfncr) if (mincore(pa, st.st_size, vec) != 0) { munmap(pa, st.st_size); -#else - /* Affect vec with mincore */ - if (fincore(fd, 0, st.st_size, vec) != 0) - { -#endif - free(vec); - FreeFile(fp); -#ifndef HAVE_FINCORE elog(ERROR, "mincore(%p, %lld, %p): %s\n", pa, (long long int)st.st_size, vec, strerror(errno)); #else + /* Affect vec with fincore */ + if (fincore(fd, 0, st.st_size, vec) != 0) + { elog(ERROR, "fincore(%u, 0, %lld, %p): %s\n", fd, (long long int)st.st_size, vec, strerror(errno)); #endif + free(vec); + FreeFile(fp); return 5; } /* * prepare the bit string */ -#ifndef HAVE_FINCORE - bitlen = (st.st_size+pgfncr->pageSize-1)/pgfncr->pageSize; -#else - bitlen = 2 * ((st.st_size+pgfncr->pageSize-1)/pgfncr->pageSize); -#endif + bitlen = FINCORE_BITS * ((st.st_size+pgfncr->pageSize-1)/pgfncr->pageSize); len = VARBITTOTALLEN(bitlen); /* * set to 0 so that *r is always initialised and string is zero-padded @@ -897,19 +875,20 @@ pgfincore_file(char *filename, pgfincoreStruct *pgfncr) { pgfncr->pages_mem++; *r |= x; -#ifdef HAVE_FINCORE - if (vec[pageIndex] & FINCORE_DIRTY) + if (FINCORE_BITS > 1) { - pgfncr->pages_dirty++; - *r |= (x >> 1); - /* we flag to detect contigous blocks in the same state */ - if (flag_dirty) - pgfncr->group_dirty++; - flag_dirty = 0; + if (vec[pageIndex] & FINCORE_DIRTY) + { + pgfncr->pages_dirty++; + *r |= (x >> 1); + /* we flag to detect contigous blocks in the same state */ + if (flag_dirty) + pgfncr->group_dirty++; + flag_dirty = 0; + } + else + flag_dirty = 1; } - else - flag_dirty = 1; -#endif elog (DEBUG5, "in memory blocks : %lld / %lld", (long long int) pageIndex, (long long int) pgfncr->rel_os_pages); @@ -922,10 +901,7 @@ pgfincore_file(char *filename, pgfincoreStruct *pgfncr) flag=1; - x >>= 1; -#ifdef HAVE_FINCORE - x >>= 1; -#endif + x >>= FINCORE_BITS; if (x == 0) { x = HIGHBIT; @@ -1103,12 +1079,10 @@ pgfincore(PG_FUNCTION_ARGS) nulls[7] = true; values[7] = (Datum) NULL; } -#ifdef HAVE_FINCORE /* number of pages dirty in OS cache */ values[8] = Int64GetDatum(pgfncr->pages_dirty); /* number of group of contigous dirty pages in os cache */ values[9] = Int64GetDatum(pgfncr->group_dirty); -#endif /* Build the result tuple. */ tuple = heap_form_tuple(fctx->tupd, values, nulls); @@ -1120,7 +1094,6 @@ pgfincore(PG_FUNCTION_ARGS) } } -#ifdef HAVE_FINCORE /* * pgfincore_drawer A very naive renderer. (for testing) */ @@ -1141,7 +1114,7 @@ pgfincore_drawer(PG_FUNCTION_ARGS) databit = PG_GETARG_VARBIT_P(0); len = VARBITLEN(databit); - result = (char *) palloc((len/2) + 1); + result = (char *) palloc((len/FINCORE_BITS) + 1); sp = VARBITS(databit); r = result; @@ -1149,15 +1122,18 @@ pgfincore_drawer(PG_FUNCTION_ARGS) { x = *sp; /* Is this bit set ? */ - for (k = 0; k < (BITS_PER_BYTE/2); k++) + for (k = 0; k < (BITS_PER_BYTE/FINCORE_BITS); k++) { char out = ' '; if (IS_HIGHBIT_SET(x)) out = '.' ; x <<= 1; - if (IS_HIGHBIT_SET(x)) - out = '*'; - x <<= 1; + if (FINCORE_BITS > 1) + { + if (IS_HIGHBIT_SET(x)) + out = '*'; + x <<= 1; + } *r++ = out; } } @@ -1165,22 +1141,22 @@ pgfincore_drawer(PG_FUNCTION_ARGS) { /* print the last partial byte */ x = *sp; - for (k = i; k < (len/2); k++) + for (k = i; k < (len/FINCORE_BITS); k++) { char out = ' '; if (IS_HIGHBIT_SET(x)) out = '.' ; x <<= 1; - if (IS_HIGHBIT_SET(x)) - out = '*'; - x <<= 1; + if (FINCORE_BITS > 1) + { + if (IS_HIGHBIT_SET(x)) + out = '*'; + x <<= 1; + } *r++ = out; } } - *r = '\0'; - PG_RETURN_CSTRING(result); + PG_RETURN_CSTRING(result); } - -#endif diff --git a/sql/pgfincore.ext.sql b/sql/pgfincore.ext.sql index c4ca50a..5a5b28f 100644 --- a/sql/pgfincore.ext.sql +++ b/sql/pgfincore.ext.sql @@ -47,3 +47,7 @@ select true from pgfadvise_sequential('test'); select true from pgfadvise_random('test'); select true from pgfadvise_normal('test'); +-- +-- tests drawers +-- +select NULL || pgfincore_drawer(databit) from pgfincore('test','main',true); diff --git a/uninstall_pgfincore.sql b/uninstall_pgfincore.sql index 99c627b..486efe5 100644 --- a/uninstall_pgfincore.sql +++ b/uninstall_pgfincore.sql @@ -15,4 +15,4 @@ DROP FUNCTION pgfadvise_loader(regclass, int, bool, bool, varbit); DROP FUNCTION pgfincore(regclass); DROP FUNCTION pgfincore(regclass, bool); DROP FUNCTION pgfincore(regclass, text, bool); - +DROP FUNCTION pgfincore_drawer(varbit); -- 2.39.5