Hi Heikki,
I'm still work on performance test, but I have following
comments/questions/suggestion:
1)
#define NodesPerPage (BLCKSZ - SizeOfPageHeaderData - offsetof(FSMPageData,
fp_nodes))
should be
#define NodesPerPage (BLCKSZ - MAXALIGN(SizeOfPageHeaderData) -
offsetof(FSMPageData, fp_nodes))
See how PageGetContents is defined
2) I suggest to renema following functions:
GetParentNo -> FSMGetParentPageNo
GetChildNo -> FSMGetChildPageNo
GetFSMBlockNumber -> FSMGetBlockNumber
3) I'm not happy much with idea that page contains data and they are
"invisible". special, lower or upper is unset. It seems like empty page. I know
that it is used in hash index implementation as well, but it could be fixed too.
I suggest to set special and upper correctly (or only upper). lower should
indicate that there are not linp.
4) I suggest to create structure
struct foo {
int level;
int logpageno;
int slot;
}
5) I see potential infinite recursive loop in fsm_search.
6) Does FANOUT^4 fit into int? (by the way what FANOUT means?)
And there are more comments on rcodereview:
pgsql/src/backend/catalog/index.c
<http://reviewdemo.postgresql.org/r/27/#comment45>
Strange comment? Looks like copy paste error.
pgsql/src/backend/catalog/index.c
<http://reviewdemo.postgresql.org/r/27/#comment47>
?RELKIND_INDEX?
pgsql/src/backend/storage/buffer/bufmgr.c
<http://reviewdemo.postgresql.org/r/27/#comment40>
Extra space
pgsql/src/backend/storage/buffer/bufmgr.c
<http://reviewdemo.postgresql.org/r/27/#comment41>
Extra space
pgsql/src/backend/storage/buffer/bufmgr.c
<http://reviewdemo.postgresql.org/r/27/#comment42>
Extra space
pgsql/src/backend/storage/buffer/bufmgr.c
<http://reviewdemo.postgresql.org/r/27/#comment43>
Extra space
pgsql/src/backend/storage/buffer/bufmgr.c
<http://reviewdemo.postgresql.org/r/27/#comment44>
Extra space
pgsql/src/backend/storage/freespace/freespace.c
<http://reviewdemo.postgresql.org/r/27/#comment37>
Use shift, however compileer could optimize it anyway.
pgsql/src/backend/storage/freespace/freespace.c
<http://reviewdemo.postgresql.org/r/27/#comment38>
Why? ;-)
pgsql/src/backend/storage/freespace/freespace.c
<http://reviewdemo.postgresql.org/r/27/#comment39>
What's happen if FSM_FORKNUM does not exist?
pgsql/src/include/storage/bufmgr.h
<http://reviewdemo.postgresql.org/r/27/#comment36>
Need consolidate - forknum vs blockNum, zeroPage
pgsql/src/include/storage/freespace.h
<http://reviewdemo.postgresql.org/r/27/#comment35>
Cleanup
pgsql/src/include/storage/lwlock.h
<http://reviewdemo.postgresql.org/r/27/#comment49>
Maybe better to use RESERVED to preserve lock numbers. It helps to DTrace
script be more backward compatible.
--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
