Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fdfaccf

Browse files
committed
Cosmetic improvements to freeze map code.
Per post-commit review comments from Andres Freund, improve variable names, comments, and in one place, slightly improve the code structure. Masahiko Sawada
1 parent a3b3076 commit fdfaccf

File tree

3 files changed

+31
-31
lines changed

3 files changed

+31
-31
lines changed

src/backend/access/heap/visibilitymap.c

+19-15
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
* is set, we know the condition is true, but if a bit is not set, it might or
3434
* might not be true.
3535
*
36-
* Clearing both visibility map bits is not separately WAL-logged. The callers
36+
* Clearing visibility map bits is not separately WAL-logged. The callers
3737
* must make sure that whenever a bit is cleared, the bit is cleared on WAL
3838
* replay of the updating operation as well.
3939
*
@@ -104,13 +104,16 @@
104104
*/
105105
#define MAPSIZE (BLCKSZ - MAXALIGN(SizeOfPageHeaderData))
106106

107+
/* Number of heap blocks we can represent in one byte */
108+
#define HEAPBLOCKS_PER_BYTE (BITS_PER_BYTE / BITS_PER_HEAPBLOCK)
109+
107110
/* Number of heap blocks we can represent in one visibility map page. */
108111
#define HEAPBLOCKS_PER_PAGE (MAPSIZE * HEAPBLOCKS_PER_BYTE)
109112

110113
/* Mapping from heap block number to the right bit in the visibility map */
111114
#define HEAPBLK_TO_MAPBLOCK(x) ((x) / HEAPBLOCKS_PER_PAGE)
112115
#define HEAPBLK_TO_MAPBYTE(x) (((x) % HEAPBLOCKS_PER_PAGE) / HEAPBLOCKS_PER_BYTE)
113-
#define HEAPBLK_TO_MAPBIT(x) (((x) % HEAPBLOCKS_PER_BYTE) * BITS_PER_HEAPBLOCK)
116+
#define HEAPBLK_TO_OFFSET(x) (((x) % HEAPBLOCKS_PER_BYTE) * BITS_PER_HEAPBLOCK)
114117

115118
/* tables for fast counting of set bits for visible and frozen */
116119
static const uint8 number_of_ones_for_visible[256] = {
@@ -156,7 +159,7 @@ static void vm_extend(Relation rel, BlockNumber nvmblocks);
156159

157160

158161
/*
159-
* visibilitymap_clear - clear all bits in visibility map
162+
* visibilitymap_clear - clear all bits for one page in visibility map
160163
*
161164
* You must pass a buffer containing the correct map page to this function.
162165
* Call visibilitymap_pin first to pin the right one. This function doesn't do
@@ -167,8 +170,8 @@ visibilitymap_clear(Relation rel, BlockNumber heapBlk, Buffer buf)
167170
{
168171
BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk);
169172
int mapByte = HEAPBLK_TO_MAPBYTE(heapBlk);
170-
int mapBit = HEAPBLK_TO_MAPBIT(heapBlk);
171-
uint8 mask = VISIBILITYMAP_VALID_BITS << mapBit;
173+
int mapOffset = HEAPBLK_TO_OFFSET(heapBlk);
174+
uint8 mask = VISIBILITYMAP_VALID_BITS << mapOffset;
172175
char *map;
173176

174177
#ifdef TRACE_VISIBILITYMAP
@@ -267,7 +270,7 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
267270
{
268271
BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk);
269272
uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk);
270-
uint8 mapBit = HEAPBLK_TO_MAPBIT(heapBlk);
273+
uint8 mapOffset = HEAPBLK_TO_OFFSET(heapBlk);
271274
Page page;
272275
uint8 *map;
273276

@@ -291,11 +294,11 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
291294
map = (uint8 *)PageGetContents(page);
292295
LockBuffer(vmBuf, BUFFER_LOCK_EXCLUSIVE);
293296

294-
if (flags != (map[mapByte] >> mapBit & VISIBILITYMAP_VALID_BITS))
297+
if (flags != (map[mapByte] >> mapOffset & VISIBILITYMAP_VALID_BITS))
295298
{
296299
START_CRIT_SECTION();
297300

298-
map[mapByte] |= (flags << mapBit);
301+
map[mapByte] |= (flags << mapOffset);
299302
MarkBufferDirty(vmBuf);
300303

301304
if (RelationNeedsWAL(rel))
@@ -338,8 +341,7 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
338341
* earlier call to visibilitymap_pin or visibilitymap_get_status on the same
339342
* relation. On return, *buf is a valid buffer with the map page containing
340343
* the bit for heapBlk, or InvalidBuffer. The caller is responsible for
341-
* releasing *buf after it's done testing and setting bits, and must pass flags
342-
* for which it needs to check the value in visibility map.
344+
* releasing *buf after it's done testing and setting bits.
343345
*
344346
* NOTE: This function is typically called without a lock on the heap page,
345347
* so somebody else could change the bit just after we look at it. In fact,
@@ -353,8 +355,9 @@ visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer *buf)
353355
{
354356
BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk);
355357
uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk);
356-
uint8 mapBit = HEAPBLK_TO_MAPBIT(heapBlk);
358+
uint8 mapOffset = HEAPBLK_TO_OFFSET(heapBlk);
357359
char *map;
360+
uint8 result;
358361

359362
#ifdef TRACE_VISIBILITYMAP
360363
elog(DEBUG1, "vm_get_status %s %d", RelationGetRelationName(rel), heapBlk);
@@ -384,7 +387,8 @@ visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer *buf)
384387
* here, but for performance reasons we make it the caller's job to worry
385388
* about that.
386389
*/
387-
return ((map[mapByte] >> mapBit) & VISIBILITYMAP_VALID_BITS);
390+
result = ((map[mapByte] >> mapOffset) & VISIBILITYMAP_VALID_BITS);
391+
return result;
388392
}
389393

390394
/*
@@ -456,7 +460,7 @@ visibilitymap_truncate(Relation rel, BlockNumber nheapblocks)
456460
/* last remaining block, byte, and bit */
457461
BlockNumber truncBlock = HEAPBLK_TO_MAPBLOCK(nheapblocks);
458462
uint32 truncByte = HEAPBLK_TO_MAPBYTE(nheapblocks);
459-
uint8 truncBit = HEAPBLK_TO_MAPBIT(nheapblocks);
463+
uint8 truncOffset = HEAPBLK_TO_OFFSET(nheapblocks);
460464

461465
#ifdef TRACE_VISIBILITYMAP
462466
elog(DEBUG1, "vm_truncate %s %d", RelationGetRelationName(rel), nheapblocks);
@@ -478,7 +482,7 @@ visibilitymap_truncate(Relation rel, BlockNumber nheapblocks)
478482
* because we don't get a chance to clear the bits if the heap is extended
479483
* again.
480484
*/
481-
if (truncByte != 0 || truncBit != 0)
485+
if (truncByte != 0 || truncOffset != 0)
482486
{
483487
Buffer mapBuffer;
484488
Page page;
@@ -511,7 +515,7 @@ visibilitymap_truncate(Relation rel, BlockNumber nheapblocks)
511515
* ((1 << 7) - 1) = 01111111
512516
*----
513517
*/
514-
map[truncByte] &= (1 << truncBit) - 1;
518+
map[truncByte] &= (1 << truncOffset) - 1;
515519

516520
MarkBufferDirty(mapBuffer);
517521
UnlockReleaseBuffer(mapBuffer);

src/backend/commands/vacuumlazy.c

+11-15
Original file line numberDiff line numberDiff line change
@@ -1192,9 +1192,9 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
11921192
}
11931193

11941194
/*
1195-
* If the page is marked as all-visible but not all-frozen, we should
1196-
* so mark it. Note that all_frozen is only valid if all_visible is
1197-
* true, so we must check both.
1195+
* If the all-visible page is turned out to be all-frozen but not marked,
1196+
* we should so mark it. Note that all_frozen is only valid if all_visible
1197+
* is true, so we must check both.
11981198
*/
11991199
else if (all_visible_according_to_vm && all_visible && all_frozen &&
12001200
!VM_ALL_FROZEN(onerel, blkno, &vmbuffer))
@@ -2068,6 +2068,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
20682068
if (ItemIdIsDead(itemid))
20692069
{
20702070
all_visible = false;
2071+
*all_frozen = false;
20712072
break;
20722073
}
20732074

@@ -2087,6 +2088,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
20872088
if (!HeapTupleHeaderXminCommitted(tuple.t_data))
20882089
{
20892090
all_visible = false;
2091+
*all_frozen = false;
20902092
break;
20912093
}
20922094

@@ -2098,6 +2100,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
20982100
if (!TransactionIdPrecedes(xmin, OldestXmin))
20992101
{
21002102
all_visible = false;
2103+
*all_frozen = false;
21012104
break;
21022105
}
21032106

@@ -2116,23 +2119,16 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
21162119
case HEAPTUPLE_RECENTLY_DEAD:
21172120
case HEAPTUPLE_INSERT_IN_PROGRESS:
21182121
case HEAPTUPLE_DELETE_IN_PROGRESS:
2119-
all_visible = false;
2120-
break;
2121-
2122+
{
2123+
all_visible = false;
2124+
*all_frozen = false;
2125+
break;
2126+
}
21222127
default:
21232128
elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
21242129
break;
21252130
}
21262131
} /* scan along page */
21272132

2128-
/*
2129-
* We don't bother clearing *all_frozen when the page is discovered not to
2130-
* be all-visible, so do that now if necessary. The page might fail to be
2131-
* all-frozen for other reasons anyway, but if it's not all-visible, then
2132-
* it definitely isn't all-frozen.
2133-
*/
2134-
if (!all_visible)
2135-
*all_frozen = false;
2136-
21372133
return all_visible;
21382134
}

src/include/access/visibilitymap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
#include "storage/buf.h"
2020
#include "utils/relcache.h"
2121

22+
/* Number of bits for one heap page */
2223
#define BITS_PER_HEAPBLOCK 2
23-
#define HEAPBLOCKS_PER_BYTE (BITS_PER_BYTE / BITS_PER_HEAPBLOCK)
2424

2525
/* Flags for bit map */
2626
#define VISIBILITYMAP_ALL_VISIBLE 0x01

0 commit comments

Comments
 (0)