Message-ID: <e54bfec11c59689890f277722aaaabd05f78e22c.camel@j-davis.com>
Subject: Add LogicalTapeSetExtend() to logtape.c
From: Jeff Davis <pgsql@j-davis.com>
To: pgsql-hackers@postgresql.org
Cc: Peter Geoghegan <pg@bowt.ie>, Heikki Linnakangas
 <heikki.linnakangas@iki.fi>,  Adam Lee <ali@pivotal.io>
Date: Thu, 27 Feb 2020 13:01:08 -0800
Content-Type: multipart/mixed; boundary="=-Lw9r7Ta14o3BbhwVz2tb"
Mime-Version: 1.0
Precedence: bulk


--=-Lw9r7Ta14o3BbhwVz2tb
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

Attached is a patch that exports a new function from logtape.c:

   extern LogicalTapeSet *LogicalTapeSetExtend(
            LogicalTapeSet *lts, int nAdditional);

The purpose is to allow adding new tapes dynamically for the upcoming
Hash Aggregation work[0]. HashAgg doesn't know in advance how many
tapes it will need, though only a limited number are actually active at
a time.

This was proposed and originally written by Adam Lee[1] (extract only
the changes to logtape.c/h from his posted patch). Strangely, I'm
seeing ~5% regression with his version when doing:

    -- t20m_1_int4 has 20 million random integers
    select * from t20m_1_int4 order by i offset 100000000;

Which seems to be due to using a pointer rather than a flexible array
member (I'm using gcc[2]). My version (attached) still uses a flexible
array member, which doesn't see the regression; but I repalloc the
whole struct so the caller needs to save the new pointer to the tape
set.

That doesn't entirely make sense to me, and I'm wondering if someone
else can repro that result and/or make a suggestion, because I don't
have a good explanation. I'm fine with my version of the patch, but it
would be nice to know why there's such a big difference using a pointer
versus a flexible array member.

Regards,
	Jeff Davis

[0] 
https://postgr.es/m/6e7c269b9a84ff75fefcad8ab2d4758f03581e98.camel%40j-davis.com
[1] https://postgr.es/m/20200108071202.GA1511@mars.local
[2] gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0


--=-Lw9r7Ta14o3BbhwVz2tb
Content-Disposition: attachment; filename="logtape.patch"
Content-Type: text/x-patch; name="logtape.patch"; charset="UTF-8"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL3NvcnQvbG9ndGFwZS5jIGIvc3JjL2JhY2tl
bmQvdXRpbHMvc29ydC9sb2d0YXBlLmMKaW5kZXggNGY3OGI1NWZiYWYuLjM2MTA0YTczYTc1IDEw
MDY0NAotLS0gYS9zcmMvYmFja2VuZC91dGlscy9zb3J0L2xvZ3RhcGUuYworKysgYi9zcmMvYmFj
a2VuZC91dGlscy9zb3J0L2xvZ3RhcGUuYwpAQCAtMjAxLDYgKzIwMSw3IEBAIHN0YXRpYyBsb25n
IGx0c0dldEZyZWVCbG9jayhMb2dpY2FsVGFwZVNldCAqbHRzKTsKIHN0YXRpYyB2b2lkIGx0c1Jl
bGVhc2VCbG9jayhMb2dpY2FsVGFwZVNldCAqbHRzLCBsb25nIGJsb2NrbnVtKTsKIHN0YXRpYyB2
b2lkIGx0c0NvbmNhdFdvcmtlclRhcGVzKExvZ2ljYWxUYXBlU2V0ICpsdHMsIFRhcGVTaGFyZSAq
c2hhcmVkLAogCQkJCQkJCQkgU2hhcmVkRmlsZVNldCAqZmlsZXNldCk7CitzdGF0aWMgdm9pZCBs
dHNJbml0VGFwZShMb2dpY2FsVGFwZSAqbHQpOwogc3RhdGljIHZvaWQgbHRzSW5pdFJlYWRCdWZm
ZXIoTG9naWNhbFRhcGVTZXQgKmx0cywgTG9naWNhbFRhcGUgKmx0KTsKIAogCkBAIC01MzYsNiAr
NTM3LDMwIEBAIGx0c0NvbmNhdFdvcmtlclRhcGVzKExvZ2ljYWxUYXBlU2V0ICpsdHMsIFRhcGVT
aGFyZSAqc2hhcmVkLAogCWx0cy0+bkhvbGVCbG9ja3MgPSBsdHMtPm5CbG9ja3NBbGxvY2F0ZWQg
LSBucGh5c2ljYWxibG9ja3M7CiB9CiAKKy8qCisgKiBJbml0aWFsaXplIHBlci10YXBlIHN0cnVj
dC4gIE5vdGUgd2UgYWxsb2NhdGUgdGhlIEkvTyBidWZmZXIgYW5kIHRoZSBmaXJzdAorICogYmxv
Y2sgZm9yIGEgdGFwZSBvbmx5IHdoZW4gaXQgaXMgZmlyc3QgYWN0dWFsbHkgd3JpdHRlbiB0by4g
IFRoaXMgYXZvaWRzCisgKiB3YXN0aW5nIG1lbW9yeSBzcGFjZSB3aGVuIHR1cGxlc29ydC5jIG92
ZXJlc3RpbWF0ZXMgdGhlIG51bWJlciBvZiB0YXBlcworICogbmVlZGVkLgorICovCitzdGF0aWMg
dm9pZAorbHRzSW5pdFRhcGUoTG9naWNhbFRhcGUgKmx0KQoreworCWx0LT53cml0aW5nICAgICAg
ICAgICA9IHRydWU7CisJbHQtPmZyb3plbiAgICAgICAgICAgID0gZmFsc2U7CisJbHQtPmRpcnR5
ICAgICAgICAgICAgID0gZmFsc2U7CisJbHQtPmZpcnN0QmxvY2tOdW1iZXIgID0gLTFMOworCWx0
LT5jdXJCbG9ja051bWJlciAgICA9IC0xTDsKKwlsdC0+bmV4dEJsb2NrTnVtYmVyICAgPSAtMUw7
CisJbHQtPm9mZnNldEJsb2NrTnVtYmVyID0gMEw7CisJbHQtPmJ1ZmZlciAgICAgICAgICAgID0g
TlVMTDsKKwlsdC0+YnVmZmVyX3NpemUgICAgICAgPSAwOworCS8qIHBhbGxvYygpIGxhcmdlciB0
aGFuIE1heEFsbG9jU2l6ZSB3b3VsZCBmYWlsICovCisJbHQtPm1heF9zaXplICAgICAgICAgID0g
TWF4QWxsb2NTaXplOworCWx0LT5wb3MgICAgICAgICAgICAgICA9IDA7CisJbHQtPm5ieXRlcyAg
ICAgICAgICAgID0gMDsKK30KKwogLyoKICAqIExhemlseSBhbGxvY2F0ZSBhbmQgaW5pdGlhbGl6
ZSB0aGUgcmVhZCBidWZmZXIuIFRoaXMgYXZvaWRzIHdhc3RlIHdoZW4gbWFueQogICogdGFwZXMg
YXJlIG9wZW4gYXQgb25jZSwgYnV0IG5vdCBhbGwgYXJlIGFjdGl2ZSBiZXR3ZWVuIHJld2luZGlu
ZyBhbmQKQEAgLTU3OSw3ICs2MDQsNiBAQCBMb2dpY2FsVGFwZVNldENyZWF0ZShpbnQgbnRhcGVz
LCBUYXBlU2hhcmUgKnNoYXJlZCwgU2hhcmVkRmlsZVNldCAqZmlsZXNldCwKIAkJCQkJIGludCB3
b3JrZXIpCiB7CiAJTG9naWNhbFRhcGVTZXQgKmx0czsKLQlMb2dpY2FsVGFwZSAqbHQ7CiAJaW50
CQkJaTsKIAogCS8qCkBAIC01OTcsMjkgKzYyMSw4IEBAIExvZ2ljYWxUYXBlU2V0Q3JlYXRlKGlu
dCBudGFwZXMsIFRhcGVTaGFyZSAqc2hhcmVkLCBTaGFyZWRGaWxlU2V0ICpmaWxlc2V0LAogCWx0
cy0+bkZyZWVCbG9ja3MgPSAwOwogCWx0cy0+blRhcGVzID0gbnRhcGVzOwogCi0JLyoKLQkgKiBJ
bml0aWFsaXplIHBlci10YXBlIHN0cnVjdHMuICBOb3RlIHdlIGFsbG9jYXRlIHRoZSBJL08gYnVm
ZmVyIGFuZCB0aGUKLQkgKiBmaXJzdCBibG9jayBmb3IgYSB0YXBlIG9ubHkgd2hlbiBpdCBpcyBm
aXJzdCBhY3R1YWxseSB3cml0dGVuIHRvLiAgVGhpcwotCSAqIGF2b2lkcyB3YXN0aW5nIG1lbW9y
eSBzcGFjZSB3aGVuIHR1cGxlc29ydC5jIG92ZXJlc3RpbWF0ZXMgdGhlIG51bWJlcgotCSAqIG9m
IHRhcGVzIG5lZWRlZC4KLQkgKi8KIAlmb3IgKGkgPSAwOyBpIDwgbnRhcGVzOyBpKyspCi0Jewot
CQlsdCA9ICZsdHMtPnRhcGVzW2ldOwotCQlsdC0+d3JpdGluZyA9IHRydWU7Ci0JCWx0LT5mcm96
ZW4gPSBmYWxzZTsKLQkJbHQtPmRpcnR5ID0gZmFsc2U7Ci0JCWx0LT5maXJzdEJsb2NrTnVtYmVy
ID0gLTFMOwotCQlsdC0+Y3VyQmxvY2tOdW1iZXIgPSAtMUw7Ci0JCWx0LT5uZXh0QmxvY2tOdW1i
ZXIgPSAtMUw7Ci0JCWx0LT5vZmZzZXRCbG9ja051bWJlciA9IDBMOwotCQlsdC0+YnVmZmVyID0g
TlVMTDsKLQkJbHQtPmJ1ZmZlcl9zaXplID0gMDsKLQkJLyogcGFsbG9jKCkgbGFyZ2VyIHRoYW4g
TWF4QWxsb2NTaXplIHdvdWxkIGZhaWwgKi8KLQkJbHQtPm1heF9zaXplID0gTWF4QWxsb2NTaXpl
OwotCQlsdC0+cG9zID0gMDsKLQkJbHQtPm5ieXRlcyA9IDA7Ci0JfQorCQlsdHNJbml0VGFwZSgm
bHRzLT50YXBlc1tpXSk7CiAKIAkvKgogCSAqIENyZWF0ZSB0ZW1wIEJ1ZkZpbGUgc3RvcmFnZSBh
cyByZXF1aXJlZC4KQEAgLTEwMDQsNiArMTAwNywyOSBAQCBMb2dpY2FsVGFwZUZyZWV6ZShMb2dp
Y2FsVGFwZVNldCAqbHRzLCBpbnQgdGFwZW51bSwgVGFwZVNoYXJlICpzaGFyZSkKIAl9CiB9CiAK
Ky8qCisgKiBBZGQgYWRkaXRpb25hbCB0YXBlcyB0byB0aGlzIHRhcGUgc2V0LiBOb3QgaW50ZW5k
ZWQgdG8gYmUgdXNlZCB3aGVuIGFueQorICogdGFwZXMgYXJlIGZyb3plbi4KKyAqLworTG9naWNh
bFRhcGVTZXQgKgorTG9naWNhbFRhcGVTZXRFeHRlbmQoTG9naWNhbFRhcGVTZXQgKmx0cywgaW50
IG5BZGRpdGlvbmFsKQoreworCWludCAgICAgaTsKKwlpbnQJCW5UYXBlc09yaWcgPSBsdHMtPm5U
YXBlczsKKwlTaXplCW5ld1NpemU7CisKKwlsdHMtPm5UYXBlcyArPSBuQWRkaXRpb25hbDsKKwlu
ZXdTaXplID0gb2Zmc2V0b2YoTG9naWNhbFRhcGVTZXQsIHRhcGVzKSArCisJCWx0cy0+blRhcGVz
ICogc2l6ZW9mKExvZ2ljYWxUYXBlKTsKKworCWx0cyA9IChMb2dpY2FsVGFwZVNldCAqKSByZXBh
bGxvYyhsdHMsIG5ld1NpemUpOworCisJZm9yIChpID0gblRhcGVzT3JpZzsgaSA8IGx0cy0+blRh
cGVzOyBpKyspCisJCWx0c0luaXRUYXBlKCZsdHMtPnRhcGVzW2ldKTsKKworCXJldHVybiBsdHM7
Cit9CisKIC8qCiAgKiBCYWNrc3BhY2UgdGhlIHRhcGUgYSBnaXZlbiBudW1iZXIgb2YgYnl0ZXMu
ICAoV2UgYWxzbyBzdXBwb3J0IGEgbW9yZQogICogZ2VuZXJhbCBzZWVrIGludGVyZmFjZSwgc2Vl
IGJlbG93LikKZGlmZiAtLWdpdCBhL3NyYy9pbmNsdWRlL3V0aWxzL2xvZ3RhcGUuaCBiL3NyYy9p
bmNsdWRlL3V0aWxzL2xvZ3RhcGUuaAppbmRleCA2OTVkMmMwMGVlNC4uM2ViZTUyMjM5ZjggMTAw
NjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL3V0aWxzL2xvZ3RhcGUuaAorKysgYi9zcmMvaW5jbHVkZS91
dGlscy9sb2d0YXBlLmgKQEAgLTY3LDYgKzY3LDggQEAgZXh0ZXJuIHZvaWQgTG9naWNhbFRhcGVS
ZXdpbmRGb3JSZWFkKExvZ2ljYWxUYXBlU2V0ICpsdHMsIGludCB0YXBlbnVtLAogZXh0ZXJuIHZv
aWQgTG9naWNhbFRhcGVSZXdpbmRGb3JXcml0ZShMb2dpY2FsVGFwZVNldCAqbHRzLCBpbnQgdGFw
ZW51bSk7CiBleHRlcm4gdm9pZCBMb2dpY2FsVGFwZUZyZWV6ZShMb2dpY2FsVGFwZVNldCAqbHRz
LCBpbnQgdGFwZW51bSwKIAkJCQkJCQkgIFRhcGVTaGFyZSAqc2hhcmUpOworZXh0ZXJuIExvZ2lj
YWxUYXBlU2V0ICpMb2dpY2FsVGFwZVNldEV4dGVuZChMb2dpY2FsVGFwZVNldCAqbHRzLAorCQkJ
CQkJCQkJCQlpbnQgbkFkZGl0aW9uYWwpOwogZXh0ZXJuIHNpemVfdCBMb2dpY2FsVGFwZUJhY2tz
cGFjZShMb2dpY2FsVGFwZVNldCAqbHRzLCBpbnQgdGFwZW51bSwKIAkJCQkJCQkJICAgc2l6ZV90
IHNpemUpOwogZXh0ZXJuIHZvaWQgTG9naWNhbFRhcGVTZWVrKExvZ2ljYWxUYXBlU2V0ICpsdHMs
IGludCB0YXBlbnVtLAo=


--=-Lw9r7Ta14o3BbhwVz2tb--