From 64cba8ff08e5f5a5f0bac8043778ed7bdab5b004 Mon Sep 17 00:00:00 2001 From: Paul Ramsey Date: Wed, 20 Sep 2017 19:58:14 +0000 Subject: [PATCH] Improve TWKT performance by avoiding lots of allocations on the heap. (Closes #3855) git-svn-id: http://svn.osgeo.org/postgis/trunk@15780 b70326c6-7e19-0410-871a-916f4a2858ee --- liblwgeom/bytebuffer.c | 97 +++++++++++++++++++++++++++++++----------- liblwgeom/bytebuffer.h | 9 +++- liblwgeom/lwout_twkb.c | 27 ++++++------ postgis/lwgeom_inout.c | 3 -- 4 files changed, 95 insertions(+), 41 deletions(-) diff --git a/liblwgeom/bytebuffer.c b/liblwgeom/bytebuffer.c index 4428392ac..913cc96cf 100644 --- a/liblwgeom/bytebuffer.c +++ b/liblwgeom/bytebuffer.c @@ -47,11 +47,19 @@ bytebuffer_create_with_size(size_t size) bytebuffer_t *s; s = lwalloc(sizeof(bytebuffer_t)); - s->buf_start = lwalloc(size); + if ( size < BYTEBUFFER_STATICSIZE ) + { + s->capacity = BYTEBUFFER_STATICSIZE; + s->buf_start = s->buf_static; + } + else + { + s->buf_start = lwalloc(size); + s->capacity = size; + } s->readcursor = s->writecursor = s->buf_start; - s->capacity = size; - memset(s->buf_start,0,size); - LWDEBUGF(4,"We create a buffer on %p of %d bytes", s->buf_start, size); + memset(s->buf_start,0,s->capacity); + LWDEBUGF(4,"We create a buffer on %p of %d bytes", s->buf_start, s->capacity); return s; } @@ -60,12 +68,20 @@ bytebuffer_create_with_size(size_t size) * struct. Useful for allocating short-lived bytebuffers off the stack. */ void -bytebuffer_init_with_size(bytebuffer_t *b, size_t size) +bytebuffer_init_with_size(bytebuffer_t *s, size_t size) { - b->buf_start = lwalloc(size); - b->readcursor = b->writecursor = b->buf_start; - b->capacity = size; - memset(b->buf_start, 0, size); + if ( size < BYTEBUFFER_STATICSIZE ) + { + s->capacity = BYTEBUFFER_STATICSIZE; + s->buf_start = s->buf_static; + } + else + { + s->buf_start = lwalloc(size); + s->capacity = size; + } + s->readcursor = s->writecursor = s->buf_start; + memset(s->buf_start, 0, s->capacity); } /** @@ -74,20 +90,22 @@ bytebuffer_init_with_size(bytebuffer_t *b, size_t size) void bytebuffer_destroy(bytebuffer_t *s) { - LWDEBUG(2,"Entered bytebuffer_destroy"); - LWDEBUGF(4,"The buffer has used %d bytes",bytebuffer_getlength(s)); - - if ( s->buf_start ) - { - LWDEBUGF(4,"let's free buf_start %p",s->buf_start); - lwfree(s->buf_start); - LWDEBUG(4,"buf_start is freed"); - } + bytebuffer_destroy_buffer(s); if ( s ) - { lwfree(s); - LWDEBUG(4,"bytebuffer_t is freed"); - } + + return; +} + +/** +* Free the bytebuffer_t and all memory managed within it. +*/ +void +bytebuffer_destroy_buffer(bytebuffer_t *s) +{ + if ( s->buf_start != s->buf_static ) + lwfree(s->buf_start); + return; } @@ -120,6 +138,7 @@ bytebuffer_makeroom(bytebuffer_t *s, size_t size_to_add) { LWDEBUGF(2,"Entered bytebuffer_makeroom with space need of %d", size_to_add); size_t current_write_size = (s->writecursor - s->buf_start); + size_t current_read_size = (s->readcursor - s->buf_start); size_t capacity = s->capacity; size_t required_size = current_write_size + size_to_add; @@ -130,14 +149,44 @@ bytebuffer_makeroom(bytebuffer_t *s, size_t size_to_add) if ( capacity > s->capacity ) { LWDEBUGF(4,"We need to realloc more memory. New capacity is %d", capacity); - s->buf_start = lwrealloc(s->buf_start, capacity); + if ( s->buf_start == s->buf_static ) + { + s->buf_start = lwalloc(capacity); + memcpy(s->buf_start, s->buf_static, s->capacity); + } + else + { + s->buf_start = lwrealloc(s->buf_start, capacity); + } s->capacity = capacity; s->writecursor = s->buf_start + current_write_size; - s->readcursor = s->buf_start + (s->readcursor - s->buf_start); + s->readcursor = s->buf_start + current_read_size; } return; } +/** Returns a copy of the internal buffer */ +uint8_t* +bytebuffer_get_buffer_copy(const bytebuffer_t *s, size_t *buffer_length) +{ + size_t bufsz = bytebuffer_getlength(s); + uint8_t *buf = lwalloc(bufsz); + memcpy(buf, s->buf_start, bufsz); + if ( buffer_length ) + *buffer_length = bufsz; + return buf; +} + +/** Returns a read-only reference to the internal buffer */ +const uint8_t* +bytebuffer_get_buffer(const bytebuffer_t *s, size_t *buffer_length) +{ + if ( buffer_length ) + *buffer_length = bytebuffer_getlength(s); + return s->buf_start; +} + + /** * Writes a uint8_t value to the buffer */ @@ -322,7 +371,7 @@ bytebuffer_read_uvarint(bytebuffer_t *b) * Returns the length of the current buffer */ size_t -bytebuffer_getlength(bytebuffer_t *s) +bytebuffer_getlength(const bytebuffer_t *s) { return (size_t) (s->writecursor - s->buf_start); } diff --git a/liblwgeom/bytebuffer.h b/liblwgeom/bytebuffer.h index 645a5e28f..8a2a15241 100644 --- a/liblwgeom/bytebuffer.h +++ b/liblwgeom/bytebuffer.h @@ -32,7 +32,8 @@ #include "varint.h" #include "lwgeom_log.h" -#define BYTEBUFFER_STARTSIZE 128 +#define BYTEBUFFER_STARTSIZE 512 +#define BYTEBUFFER_STATICSIZE 1024 typedef struct { @@ -40,6 +41,7 @@ typedef struct uint8_t *buf_start; uint8_t *writecursor; uint8_t *readcursor; + uint8_t buf_static[BYTEBUFFER_STATICSIZE]; } bytebuffer_t; @@ -47,15 +49,18 @@ void bytebuffer_init_with_size(bytebuffer_t *b, size_t size); bytebuffer_t *bytebuffer_create_with_size(size_t size); bytebuffer_t *bytebuffer_create(void); void bytebuffer_destroy(bytebuffer_t *s); +void bytebuffer_destroy_buffer(bytebuffer_t *s); void bytebuffer_clear(bytebuffer_t *s); void bytebuffer_append_byte(bytebuffer_t *s, const uint8_t val); void bytebuffer_append_varint(bytebuffer_t *s, const int64_t val); void bytebuffer_append_uvarint(bytebuffer_t *s, const uint64_t val); uint64_t bytebuffer_read_uvarint(bytebuffer_t *s); int64_t bytebuffer_read_varint(bytebuffer_t *s); -size_t bytebuffer_getlength(bytebuffer_t *s); +size_t bytebuffer_getlength(const bytebuffer_t *s); bytebuffer_t* bytebuffer_merge(bytebuffer_t **buff_array, int nbuffers); void bytebuffer_reset_reading(bytebuffer_t *s); +uint8_t* bytebuffer_get_buffer_copy(const bytebuffer_t *s, size_t *buffer_length); +const uint8_t* bytebuffer_get_buffer(const bytebuffer_t *s, size_t *buffer_length); void bytebuffer_append_bytebuffer(bytebuffer_t *write_to,bytebuffer_t *write_from); void bytebuffer_append_bulk(bytebuffer_t *s, void * start, size_t size); diff --git a/liblwgeom/lwout_twkb.c b/liblwgeom/lwout_twkb.c index f535b4b27..38ca94046 100644 --- a/liblwgeom/lwout_twkb.c +++ b/liblwgeom/lwout_twkb.c @@ -409,13 +409,17 @@ static int lwgeom_write_to_buffer(const LWGEOM *geom, TWKB_GLOBALS *globals, TWK int i, is_empty, has_z, has_m, ndims; size_t bbox_size = 0, optional_precision_byte = 0; uint8_t flag = 0, type_prec = 0; + bytebuffer_t header_bytebuffer, geom_bytebuffer; TWKB_STATE child_state; memset(&child_state, 0, sizeof(TWKB_STATE)); - child_state.header_buf = bytebuffer_create_with_size(16); - child_state.geom_buf = bytebuffer_create_with_size(64); + child_state.header_buf = &header_bytebuffer; + child_state.geom_buf = &geom_bytebuffer; child_state.idlist = parent_state->idlist; + bytebuffer_init_with_size(child_state.header_buf, 16); + bytebuffer_init_with_size(child_state.geom_buf, 64); + /* Read dimensionality from input */ has_z = lwgeom_has_z(geom); has_m = lwgeom_has_m(geom); @@ -499,8 +503,8 @@ static int lwgeom_write_to_buffer(const LWGEOM *geom, TWKB_GLOBALS *globals, TWK bytebuffer_append_byte(child_state.header_buf, 0); bytebuffer_append_bytebuffer(parent_state->geom_buf, child_state.header_buf); - bytebuffer_destroy(child_state.header_buf); - bytebuffer_destroy(child_state.geom_buf); + bytebuffer_destroy_buffer(child_state.header_buf); + bytebuffer_destroy_buffer(child_state.geom_buf); return 0; } @@ -546,8 +550,8 @@ static int lwgeom_write_to_buffer(const LWGEOM *geom, TWKB_GLOBALS *globals, TWK bytebuffer_append_bytebuffer(parent_state->geom_buf,child_state.header_buf); bytebuffer_append_bytebuffer(parent_state->geom_buf,child_state.geom_buf); - bytebuffer_destroy(child_state.header_buf); - bytebuffer_destroy(child_state.geom_buf); + bytebuffer_destroy_buffer(child_state.header_buf); + bytebuffer_destroy_buffer(child_state.geom_buf); return 0; } @@ -566,6 +570,7 @@ lwgeom_to_twkb_with_idlist(const LWGEOM *geom, int64_t *idlist, uint8_t variant, TWKB_GLOBALS tg; TWKB_STATE ts; + bytebuffer_t geom_bytebuffer; uint8_t *twkb; @@ -592,14 +597,12 @@ lwgeom_to_twkb_with_idlist(const LWGEOM *geom, int64_t *idlist, uint8_t variant, ts.idlist = idlist; ts.header_buf = NULL; - ts.geom_buf = bytebuffer_create(); + ts.geom_buf = &geom_bytebuffer; + bytebuffer_init_with_size(ts.geom_buf, 512); lwgeom_write_to_buffer(geom, &tg, &ts); - if ( twkb_size ) - *twkb_size = bytebuffer_getlength(ts.geom_buf); - - twkb = ts.geom_buf->buf_start; - lwfree(ts.geom_buf); + twkb = bytebuffer_get_buffer_copy(ts.geom_buf, twkb_size); + bytebuffer_destroy_buffer(ts.geom_buf); return twkb; } diff --git a/postgis/lwgeom_inout.c b/postgis/lwgeom_inout.c index f67903456..5fcb3ead6 100644 --- a/postgis/lwgeom_inout.c +++ b/postgis/lwgeom_inout.c @@ -507,15 +507,12 @@ Datum TWKBFromLWGEOM(PG_FUNCTION_ARGS) /* Create TWKB binary string */ lwgeom = lwgeom_from_gserialized(geom); twkb = lwgeom_to_twkb(lwgeom, variant, sp.precision_xy, sp.precision_z, sp.precision_m, &twkb_size); - lwgeom_free(lwgeom); /* Prepare the PgSQL text return type */ result = palloc(twkb_size + VARHDRSZ); memcpy(VARDATA(result), twkb, twkb_size); SET_VARSIZE(result, twkb_size + VARHDRSZ); - pfree(twkb); - PG_FREE_IF_COPY(geom, 0); PG_RETURN_BYTEA_P(result); } -- 2.40.0