Hello, >Patches as well as results are attached.
I had a look at code. I have few minor points, + bkpb.fork_flags |= BKPBLOCK_HAS_IMAGE; + + if (is_compressed) { - rdt_datas_last->data = page; - rdt_datas_last->len = BLCKSZ; + /* compressed block information */ + bimg.length = compress_len; + bimg.extra_data = hole_offset; + bimg.extra_data |= XLR_BLCK_COMPRESSED_MASK; For consistency with the existing code , how about renaming the macro XLR_BLCK_COMPRESSED_MASK as BKPBLOCK_HAS_COMPRESSED_IMAGE on the lines of BKPBLOCK_HAS_IMAGE. + blk->hole_offset = extra_data & ~XLR_BLCK_COMPRESSED_MASK; Here , I think that having the mask as BKPBLOCK_HOLE_OFFSET_MASK will be more indicative of the fact that lower 15 bits of extra_data field comprises of hole_offset value. This suggestion is also just to achieve consistency with the existing BKPBLOCK_FORK_MASK for fork_flags field. And comment typo + * First try to compress block, filling in the page hole with zeros + * to improve the compression of the whole. If the block is considered + * as incompressible, complete the block header information as if + * nothing happened. As hole is no longer being compressed, this needs to be changed. Thank you, Rahila Syed On Tue, Dec 16, 2014 at 10:04 PM, Michael Paquier <michael.paqu...@gmail.com > wrote: > > > > On Wed, Dec 17, 2014 at 12:00 AM, Michael Paquier < > michael.paqu...@gmail.com> wrote: >> >> Actually, the original length of the compressed block in saved in >> PGLZ_Header, so if we are fine to not check the size of the block >> decompressed when decoding WAL we can do without the hole filled with >> zeros, and use only 1 bit to see if the block is compressed or not. >> > And.. After some more hacking, I have been able to come up with a patch > that is able to compress blocks without the page hole, and that keeps the > WAL record length the same as HEAD when compression switch is off. The > numbers are pretty good, CPU is saved in the same proportions as previous > patches when compression is enabled, and there is zero delta with HEAD when > compression switch is off. > > Here are the actual numbers: > test_name | pg_size_pretty | user_diff | system_diff > -------------------------------+----------------+-----------+------------- > FPW on + 2 bytes, ffactor 50 | 582 MB | 42.391894 | 0.807444 > FPW on + 2 bytes, ffactor 20 | 229 MB | 14.330304 | 0.729626 > FPW on + 2 bytes, ffactor 10 | 117 MB | 7.335442 | 0.570996 > FPW off + 2 bytes, ffactor 50 | 746 MB | 25.330391 | 1.248503 > FPW off + 2 bytes, ffactor 20 | 293 MB | 10.537475 | 0.755448 > FPW off + 2 bytes, ffactor 10 | 148 MB | 5.762775 | 0.763761 > FPW on + 0 bytes, ffactor 50 | 582 MB | 42.174297 | 0.790596 > FPW on + 0 bytes, ffactor 20 | 229 MB | 14.424233 | 0.770459 > FPW on + 0 bytes, ffactor 10 | 117 MB | 7.057195 | 0.584806 > FPW off + 0 bytes, ffactor 50 | 746 MB | 25.261998 | 1.054516 > FPW off + 0 bytes, ffactor 20 | 293 MB | 10.589888 | 0.860207 > FPW off + 0 bytes, ffactor 10 | 148 MB | 5.827191 | 0.874285 > HEAD, ffactor 50 | 746 MB | 25.181729 | 1.133433 > HEAD, ffactor 20 | 293 MB | 9.962242 | 0.765970 > HEAD, ffactor 10 | 148 MB | 5.693426 | 0.775371 > Record, ffactor 50 | 582 MB | 54.904374 | 0.678204 > Record, ffactor 20 | 229 MB | 19.798268 | 0.807220 > Record, ffactor 10 | 116 MB | 9.401877 | 0.668454 > (18 rows) > > The new tests of this patch are "FPW off + 0 bytes". Patches as well as > results are attached. > Regards, > -- > Michael >