[postgis-tickets] r15780 - Improve TWKT performance by avoiding lots of allocations

Paul Ramsey pramsey at cleverelephant.ca
Wed Sep 20 12:58:14 PDT 2017


Author: pramsey
Date: 2017-09-20 12:58:14 -0700 (Wed, 20 Sep 2017)
New Revision: 15780

Modified:
   trunk/liblwgeom/bytebuffer.c
   trunk/liblwgeom/bytebuffer.h
   trunk/liblwgeom/lwout_twkb.c
   trunk/postgis/lwgeom_inout.c
Log:
Improve TWKT performance by avoiding lots of allocations
on the heap. (Closes #3855)


Modified: trunk/liblwgeom/bytebuffer.c
===================================================================
--- trunk/liblwgeom/bytebuffer.c	2017-09-20 19:55:56 UTC (rev 15779)
+++ trunk/liblwgeom/bytebuffer.c	2017-09-20 19:58:14 UTC (rev 15780)
@@ -47,11 +47,19 @@
 	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 @@
 * 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,24 +90,26 @@
 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;
+}
+
+/**
 * Set the read cursor to the beginning
 */
 void
@@ -120,6 +138,7 @@
 {
 	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 @@
 	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 @@
 * 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);
 }

Modified: trunk/liblwgeom/bytebuffer.h
===================================================================
--- trunk/liblwgeom/bytebuffer.h	2017-09-20 19:55:56 UTC (rev 15779)
+++ trunk/liblwgeom/bytebuffer.h	2017-09-20 19:58:14 UTC (rev 15780)
@@ -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 @@
 	uint8_t *buf_start;
 	uint8_t *writecursor;
 	uint8_t *readcursor;
+	uint8_t buf_static[BYTEBUFFER_STATICSIZE];
 }
 bytebuffer_t;
 
@@ -47,15 +49,18 @@
 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);

Modified: trunk/liblwgeom/lwout_twkb.c
===================================================================
--- trunk/liblwgeom/lwout_twkb.c	2017-09-20 19:55:56 UTC (rev 15779)
+++ trunk/liblwgeom/lwout_twkb.c	2017-09-20 19:58:14 UTC (rev 15780)
@@ -409,13 +409,17 @@
 	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 @@
 			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 @@
 	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 @@
 
 	TWKB_GLOBALS tg;
 	TWKB_STATE ts;
+	bytebuffer_t geom_bytebuffer;
 
 	uint8_t *twkb;
 
@@ -592,14 +597,12 @@
 
 	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;
 }
 

Modified: trunk/postgis/lwgeom_inout.c
===================================================================
--- trunk/postgis/lwgeom_inout.c	2017-09-20 19:55:56 UTC (rev 15779)
+++ trunk/postgis/lwgeom_inout.c	2017-09-20 19:58:14 UTC (rev 15780)
@@ -507,15 +507,12 @@
 	/* 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);
 }
 



More information about the postgis-tickets mailing list