Re: [HACKERS] Custom allocators in libpq
On 29 August 2017 at 05:15, Tom Lanewrote: > Peter Eisentraut writes: > > On 8/28/17 15:11, Tom Lane wrote: > >> ... but it seems like you're giving up a lot of the possible uses if > >> you don't make it apply uniformly. I admit I'm not sure how we'd handle > >> the initial creation of a connection object with a custom malloc. The > >> obvious solution of requiring the functions to be specified at PQconnect > >> time seems to require Yet Another PQconnect Variant, which is not very > >> appetizing. > > > I would have expected a separate function just to register the callback > > functions, before doing anything else with libpq. Similar to libxml: > > http://xmlsoft.org/xmlmem.html > > I really don't much care for libxml's solution, because it implies > global variables, with the attendant thread-safety issues. That's > especially bad if you want a passthrough such as a memory context > pointer, since it's quite likely that different call sites would > need different passthrough values, even assuming that a single set > of callback functions would suffice for an entire application. > That latter assumption isn't so pleasant either. One could expect > that by using such a solution, postgres_fdw could be expected to > break, say, a libpq-based DBI library inside plperl. Yeah, the 'register a malloc() function pointer in a global via a registration function call' approach seems fine and dandy until you find yourself with an app that, via shared library loads, has more than one different user of libpq with its own ideas about memory allocation. RTLD_LOCAL can help, but may introduce other issues. So there doesn't seem much way around another PQconnect variant. Yay? We could switch to a struct-passing argument model, but by the time you add the necessary "nfields" argument to allow you to know how much of the struct you can safely access, etc, just adding new connect functions starts to look good in comparison. Which reminds me, it kind of stinks that PQconnectdbParams and PQpingParams accept key and value char* arrays, but PQconninfoParse produces a PQconninfoOption* . This makes it seriously annoying when you want to parse a connstring, make some transformations and pass it to a connect function. I pretty much always just put the user's original connstring in 'dbname' and set expand_dbname = true instead. It might make sense to have any new function accept PQconninfoOption*. Or a variant of PQconninfoParse that populates k/v arrays with 'n' extra fields allocated and zeroed on return, I guess. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] Custom allocators in libpq
Peter Eisentrautwrites: > On 8/28/17 15:11, Tom Lane wrote: >> ... but it seems like you're giving up a lot of the possible uses if >> you don't make it apply uniformly. I admit I'm not sure how we'd handle >> the initial creation of a connection object with a custom malloc. The >> obvious solution of requiring the functions to be specified at PQconnect >> time seems to require Yet Another PQconnect Variant, which is not very >> appetizing. > I would have expected a separate function just to register the callback > functions, before doing anything else with libpq. Similar to libxml: > http://xmlsoft.org/xmlmem.html I really don't much care for libxml's solution, because it implies global variables, with the attendant thread-safety issues. That's especially bad if you want a passthrough such as a memory context pointer, since it's quite likely that different call sites would need different passthrough values, even assuming that a single set of callback functions would suffice for an entire application. That latter assumption isn't so pleasant either. One could expect that by using such a solution, postgres_fdw could be expected to break, say, a libpq-based DBI library inside plperl. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Custom allocators in libpq
On 8/28/17 15:11, Tom Lane wrote: > ... but it seems like you're giving up a lot of the possible uses if > you don't make it apply uniformly. I admit I'm not sure how we'd handle > the initial creation of a connection object with a custom malloc. The > obvious solution of requiring the functions to be specified at PQconnect > time seems to require Yet Another PQconnect Variant, which is not very > appetizing. I would have expected a separate function just to register the callback functions, before doing anything else with libpq. Similar to libxml: http://xmlsoft.org/xmlmem.html -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Custom allocators in libpq
On Mon, Aug 28, 2017 at 03:11:26PM -0400, Tom Lane wrote: > Aaron Pattersonwrites: > > I would like to be able to configure libpq with custom malloc functions. > > I can see the potential value of this ... > > > This patch doesn't replace all malloc calls to the configured ones, just > > the mallocs related to creating result objects (which is what I'm > > concerned with). > > ... but it seems like you're giving up a lot of the possible uses if > you don't make it apply uniformly. I'm happy to make the changes uniformly! I'll do that and update the patch. > I admit I'm not sure how we'd handle > the initial creation of a connection object with a custom malloc. The > obvious solution of requiring the functions to be specified at PQconnect > time seems to require Yet Another PQconnect Variant, which is not very > appetizing. Other libraries I've worked with allow me to malloc a struct, then pass it to an initialization function. This might take a bit of refactoring, like introducing a new `PQconnectStart`, but might be worth while. > I also wonder whether you wouldn't want a passthrough argument. > For instance, one of the use-cases that springs to mind immediately is > teaching postgres_fdw and dblink to use this so that their result objects > are palloc'd not malloc'd, allowing removal of lots of PG_TRY overhead. > While I suppose we could have the hook functions always allocate in > CurrentMemoryContext, it'd likely be useful to be able to specify > "use context X" at creation time. We don't need this for the Ruby wrapper, but I've seen other libraries do it. I'm happy to add it as well. Thanks! -- Aaron Patterson http://tenderlovemaking.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Custom allocators in libpq
Aaron Pattersonwrites: > I would like to be able to configure libpq with custom malloc functions. I can see the potential value of this ... > This patch doesn't replace all malloc calls to the configured ones, just > the mallocs related to creating result objects (which is what I'm > concerned with). ... but it seems like you're giving up a lot of the possible uses if you don't make it apply uniformly. I admit I'm not sure how we'd handle the initial creation of a connection object with a custom malloc. The obvious solution of requiring the functions to be specified at PQconnect time seems to require Yet Another PQconnect Variant, which is not very appetizing. I also wonder whether you wouldn't want a passthrough argument. For instance, one of the use-cases that springs to mind immediately is teaching postgres_fdw and dblink to use this so that their result objects are palloc'd not malloc'd, allowing removal of lots of PG_TRY overhead. While I suppose we could have the hook functions always allocate in CurrentMemoryContext, it'd likely be useful to be able to specify "use context X" at creation time. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Custom allocators in libpq
Hello! I would like to be able to configure libpq with custom malloc functions. The reason is that we have a Ruby wrapper that exposes libpq in Ruby. The problem is that Ruby's GC doesn't know how much memory has been allocated by libpq, so no pressure is applied to the GC when it should be. Ruby exports malloc functions that automatically apply GC pressure, and I'd like to be able to configure libpq to use those malloc functions. I've attached two patches that add this functionality. The first patch introduces a new function `PQunescapeByteaConn` which takes a connection (so we have a place to get the malloc functions). We already have `PQescapeBytea` and `PQescapeByteaConn`, this first patch gives us the analogous `PQunescapeBytea` and `PQunescapeByteaConn`. The second patch adds malloc function pointer fields to `PGEvent`, `pg_result`, and `pg_conn` structs, and changes libpq internals to use those allocators rather than directly calling `malloc`. This patch doesn't replace all malloc calls to the configured ones, just the mallocs related to creating result objects (which is what I'm concerned with). If there's something I'm missing, please let me know. This is my first patch to libpq, so I look forward to hearing feedback. Thanks for your time! -- Aaron Patterson http://tenderlovemaking.com/ >From be463a2da13734d5b104a5fc9c58e7bbb8908323 Mon Sep 17 00:00:00 2001 From: Aaron PattersonDate: Thu, 19 Nov 2015 14:44:49 -0800 Subject: [PATCH 1/2] add PQunescapeByteaConn for unescaping bytes given a connection This commit adds an unescape method that takes a connection as a parameter. The connection may have settings on it (like allocators) that are important to unescaping the string. --- src/interfaces/libpq/exports.txt | 1 + src/interfaces/libpq/fe-exec.c | 21 +++-- src/interfaces/libpq/libpq-fe.h | 6 -- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index d6a38d0df8..ba718f4071 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -172,3 +172,4 @@ PQsslAttribute169 PQsetErrorContextVisibility 170 PQresultVerboseErrorMessage 171 PQencryptPasswordConn 172 +PQunescapeByteaConn 173 diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index e1e2d18e3a..464bdc635f 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -3651,8 +3651,9 @@ PQescapeBytea(const unsigned char *from, size_t from_length, size_t *to_length) * \ooo == a byte whose value = ooo (ooo is an octal number) * \x == x (x is any character not matched by the above transformations) */ -unsigned char * -PQunescapeBytea(const unsigned char *strtext, size_t *retbuflen) +static unsigned char * +PQunescapeByteaInternal(PGconn *conn, + const unsigned char *strtext, size_t *retbuflen) { size_t strtextlen, buflen; @@ -3762,3 +3763,19 @@ PQunescapeBytea(const unsigned char *strtext, size_t *retbuflen) *retbuflen = buflen; return tmpbuf; } + +unsigned char * +PQunescapeByteaConn(PGconn *conn, + const unsigned char *strtext, size_t *retbuflen) +{ + if (!conn) + return NULL; + + return PQunescapeByteaInternal(conn, strtext, retbuflen); +} + +unsigned char * +PQunescapeBytea(const unsigned char *strtext, size_t *retbuflen) +{ + return PQunescapeByteaInternal(NULL, strtext, retbuflen); +} diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 1d915e7915..7fce4b387e 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -527,13 +527,15 @@ extern char *PQescapeIdentifier(PGconn *conn, const char *str, size_t len); extern unsigned char *PQescapeByteaConn(PGconn *conn, const unsigned char *from, size_t from_length, size_t *to_length); -extern unsigned char *PQunescapeBytea(const unsigned char *strtext, - size_t *retbuflen); +extern unsigned char *PQunescapeByteaConn(PGconn *conn, + const unsigned char *strtext, size_t *retbuflen); /* These forms are deprecated! */ extern size_t PQescapeString(char *to, const char *from, size_t length); extern unsigned char *PQescapeBytea(const unsigned char *from, size_t from_length, size_t *to_length); +extern unsigned char *PQunescapeBytea(const unsigned char *strtext, + size_t *retbuflen); -- 2.11.0 >From b62fed4a7e38ef9e2057d5556312a5a76cdf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 19 Nov 2015 15:28:27 -0800 Subject: [PATCH 2/2] use configured malloc /