Skip to content

Commit 6bda0c1

Browse files
nikicnikic
committed
MySQLnd: Simplify management of zval row buffer
Something odd was being done here, with the row packet having a flag for whether it should allocate the zval buffer, which would then get moved into the result set. Keep the management of this buffer purely at the result set level. This also allows us to easily reuse the same buffer for all results, rather than allocating a new one for each fetch.
1 parent 6b37cdd commit 6bda0c1

File tree

4 files changed

+46
-105
lines changed

4 files changed

+46
-105
lines changed

‎ext/mysqlnd/mysqlnd_ps.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -841,9 +841,6 @@ mysqlnd_stmt_fetch_row_unbuffered(MYSQLND_RES * result, void * param, const unsi
841841
DBG_RETURN(FAIL);
842842
}
843843

844-
/* Let the row packet fill our buffer and skip additional malloc + memcpy */
845-
row_packet->skip_extraction = stmt && stmt->result_bind? FALSE:TRUE;
846-
847844
checkpoint = result->memory_pool->checkpoint;
848845
mysqlnd_mempool_save_state(result->memory_pool);
849846

@@ -854,12 +851,10 @@ mysqlnd_stmt_fetch_row_unbuffered(MYSQLND_RES * result, void * param, const unsi
854851
if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
855852
unsigned int i, field_count = result->field_count;
856853

857-
if (!row_packet->skip_extraction) {
854+
if (stmt && stmt->result_bind) {
858855
result->unbuf->m.free_last_data(result->unbuf, conn->stats);
859856

860-
result->unbuf->last_row_data = row_packet->fields;
861857
result->unbuf->last_row_buffer = row_packet->row_buffer;
862-
row_packet->fields = NULL;
863858
row_packet->row_buffer.ptr = NULL;
864859

865860
if (PASS != result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
@@ -1029,19 +1024,15 @@ mysqlnd_fetch_stmt_row_cursor(MYSQLND_RES * result, void * param, const unsigned
10291024

10301025
}
10311026

1032-
row_packet->skip_extraction = stmt->result_bind? FALSE:TRUE;
1033-
10341027
UPSERT_STATUS_RESET(stmt->upsert_status);
10351028
if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
10361029
const MYSQLND_RES_METADATA * const meta = result->meta;
10371030
unsigned int i, field_count = result->field_count;
10381031

1039-
if (!row_packet->skip_extraction) {
1032+
if (stmt->result_bind) {
10401033
result->unbuf->m.free_last_data(result->unbuf, conn->stats);
10411034

1042-
result->unbuf->last_row_data = row_packet->fields;
10431035
result->unbuf->last_row_buffer = row_packet->row_buffer;
1044-
row_packet->fields = NULL;
10451036
row_packet->row_buffer.ptr = NULL;
10461037

10471038
if (PASS != result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,

‎ext/mysqlnd/mysqlnd_result.c

Lines changed: 43 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -147,18 +147,12 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, free_last_data)(MYSQLND_RES_UNBUFFERED
147147
}
148148

149149
DBG_INF_FMT("field_count=%u", unbuf->field_count);
150-
if (unbuf->last_row_data) {
151-
unsigned int i;
152-
for (i = 0; i < unbuf->field_count; i++) {
153-
zval_ptr_dtor_nogc(&(unbuf->last_row_data[i]));
154-
}
155-
156-
/* Free last row's zvals */
157-
mnd_efree(unbuf->last_row_data);
158-
unbuf->last_row_data = NULL;
159-
}
160150
if (unbuf->last_row_buffer.ptr) {
161151
DBG_INF("Freeing last row buffer");
152+
for (unsigned i = 0; i < unbuf->field_count; i++) {
153+
zval_ptr_dtor_nogc(&unbuf->last_row_data[i]);
154+
}
155+
162156
/* Nothing points to this buffer now, free it */
163157
unbuf->result_set_memory_pool->free_chunk(
164158
unbuf->result_set_memory_pool, unbuf->last_row_buffer.ptr);
@@ -612,7 +606,7 @@ static const size_t *
612606
MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_lengths)(const MYSQLND_RES_UNBUFFERED * const result)
613607
{
614608
/* simulate output of libmysql */
615-
return (result->last_row_data || result->eof_reached)? result->lengths : NULL;
609+
return (result->last_row_buffer.ptr || result->eof_reached)? result->lengths : NULL;
616610
}
617611
/* }}} */
618612

@@ -660,8 +654,6 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row_c)(MYSQLND_RES * result, voi
660654
/* Not fully initialized object that is being cleaned up */
661655
DBG_RETURN(FAIL);
662656
}
663-
/* Let the row packet fill our buffer and skip additional mnd_malloc + memcpy */
664-
row_packet->skip_extraction = FALSE;
665657

666658
checkpoint = result->memory_pool->checkpoint;
667659
mysqlnd_mempool_save_state(result->memory_pool);
@@ -673,52 +665,48 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row_c)(MYSQLND_RES * result, voi
673665
if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
674666
result->unbuf->m.free_last_data(result->unbuf, conn->stats);
675667

676-
result->unbuf->last_row_data = row_packet->fields;
677668
result->unbuf->last_row_buffer = row_packet->row_buffer;
678-
row_packet->fields = NULL;
679669
row_packet->row_buffer.ptr = NULL;
680670

681671
MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_ROWS_FETCHED_FROM_CLIENT_NORMAL_UNBUF);
682672

683-
if (!row_packet->skip_extraction) {
684-
unsigned int i, field_count = meta->field_count;
673+
unsigned int i, field_count = meta->field_count;
674+
675+
enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
676+
result->unbuf->last_row_data,
677+
field_count,
678+
row_packet->fields_metadata,
679+
conn->options->int_and_float_native,
680+
conn->stats);
681+
if (PASS != rc) {
682+
mysqlnd_mempool_restore_state(result->memory_pool);
683+
result->memory_pool->checkpoint = checkpoint;
684+
DBG_RETURN(FAIL);
685+
}
686+
{
687+
*row = mnd_emalloc(field_count * sizeof(char *));
688+
MYSQLND_FIELD * field = meta->fields;
689+
size_t * lengths = result->unbuf->lengths;
685690

686-
enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
687-
result->unbuf->last_row_data,
688-
field_count,
689-
row_packet->fields_metadata,
690-
conn->options->int_and_float_native,
691-
conn->stats);
692-
if (PASS != rc) {
693-
mysqlnd_mempool_restore_state(result->memory_pool);
694-
result->memory_pool->checkpoint = checkpoint;
695-
DBG_RETURN(FAIL);
696-
}
697-
{
698-
*row = mnd_emalloc(field_count * sizeof(char *));
699-
MYSQLND_FIELD * field = meta->fields;
700-
size_t * lengths = result->unbuf->lengths;
701-
702-
for (i = 0; i < field_count; i++, field++) {
703-
zval * data = &result->unbuf->last_row_data[i];
704-
const size_t len = (Z_TYPE_P(data) == IS_STRING)? Z_STRLEN_P(data) : 0;
691+
for (i = 0; i < field_count; i++, field++) {
692+
zval * data = &result->unbuf->last_row_data[i];
693+
const size_t len = (Z_TYPE_P(data) == IS_STRING)? Z_STRLEN_P(data) : 0;
705694

706695
/* BEGIN difference between normal normal fetch and _c */
707-
if (Z_TYPE_P(data) != IS_NULL) {
708-
convert_to_string(data);
709-
(*row)[i] = Z_STRVAL_P(data);
710-
} else {
711-
(*row)[i] = NULL;
712-
}
696+
if (Z_TYPE_P(data) != IS_NULL) {
697+
convert_to_string(data);
698+
(*row)[i] = Z_STRVAL_P(data);
699+
} else {
700+
(*row)[i] = NULL;
701+
}
713702
/* END difference between normal normal fetch and _c */
714703

715-
if (lengths) {
716-
lengths[i] = len;
717-
}
704+
if (lengths) {
705+
lengths[i] = len;
706+
}
718707

719-
if (field->max_length < len) {
720-
field->max_length = len;
721-
}
708+
if (field->max_length < len) {
709+
field->max_length = len;
722710
}
723711
}
724712
}
@@ -788,8 +776,6 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row)(MYSQLND_RES * result, void
788776
/* Not fully initialized object that is being cleaned up */
789777
DBG_RETURN(FAIL);
790778
}
791-
/* Let the row packet fill our buffer and skip additional mnd_malloc + memcpy */
792-
row_packet->skip_extraction = row? FALSE:TRUE;
793779

794780
checkpoint = result->memory_pool->checkpoint;
795781
mysqlnd_mempool_save_state(result->memory_pool);
@@ -801,14 +787,12 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row)(MYSQLND_RES * result, void
801787
if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
802788
result->unbuf->m.free_last_data(result->unbuf, conn->stats);
803789

804-
result->unbuf->last_row_data = row_packet->fields;
805790
result->unbuf->last_row_buffer = row_packet->row_buffer;
806-
row_packet->fields = NULL;
807791
row_packet->row_buffer.ptr = NULL;
808792

809793
MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_ROWS_FETCHED_FROM_CLIENT_NORMAL_UNBUF);
810794

811-
if (!row_packet->skip_extraction) {
795+
if (row) {
812796
unsigned int i, field_count = meta->field_count;
813797

814798
enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
@@ -1263,8 +1247,6 @@ MYSQLND_METHOD(mysqlnd_res, store_result_fetch_data)(MYSQLND_CONN_DATA * const c
12631247
row_packet.binary_protocol = binary_protocol;
12641248
row_packet.fields_metadata = meta->fields;
12651249

1266-
row_packet.skip_extraction = TRUE; /* let php_mysqlnd_rowp_read() not allocate row_packet.fields, we will do it */
1267-
12681250
while (FAIL != (ret = PACKET_READ(conn, &row_packet)) && !row_packet.eof) {
12691251
if (!free_rows) {
12701252
MYSQLND_ROW_BUFFER * new_row_buffers;
@@ -1301,7 +1283,6 @@ MYSQLND_METHOD(mysqlnd_res, store_result_fetch_data)(MYSQLND_CONN_DATA * const c
13011283
set->row_count++;
13021284

13031285
/* So row_packet's destructor function won't efree() it */
1304-
row_packet.fields = NULL;
13051286
row_packet.row_buffer.ptr = NULL;
13061287

13071288
/*
@@ -1459,7 +1440,9 @@ MYSQLND_METHOD(mysqlnd_res, skip_result)(MYSQLND_RES * const result)
14591440
STAT_FLUSHED_PS_SETS);
14601441

14611442
while ((PASS == result->m.fetch_row(result, NULL, 0, &fetched_anything)) && fetched_anything == TRUE) {
1462-
/* do nothing */;
1443+
MYSQLND_INC_CONN_STATISTIC(conn->stats,
1444+
result->type == MYSQLND_RES_NORMAL
1445+
? STAT_ROWS_SKIPPED_NORMAL : STAT_ROWS_SKIPPED_PS);
14631446
}
14641447
}
14651448
DBG_RETURN(PASS);
@@ -1880,6 +1863,9 @@ mysqlnd_result_unbuffered_init(MYSQLND_RES *result, const unsigned int field_cou
18801863
ret->lengths = pool->get_chunk(pool, field_count * sizeof(size_t));
18811864
memset(ret->lengths, 0, field_count * sizeof(size_t));
18821865

1866+
ret->last_row_data = pool->get_chunk(pool, field_count * sizeof(zval));
1867+
memset(ret->last_row_data, 0, field_count * sizeof(zval));
1868+
18831869
ret->result_set_memory_pool = pool;
18841870
ret->field_count= field_count;
18851871
ret->ps = ps;

‎ext/mysqlnd/mysqlnd_wireprotocol.c

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,10 +1696,6 @@ php_mysqlnd_rowp_read_text_protocol_c(MYSQLND_ROW_BUFFER * row_buffer, zval * fi
16961696

16971697

16981698
/* {{{ php_mysqlnd_rowp_read */
1699-
/*
1700-
if normal statements => packet->fields is created by this function,
1701-
if PS => packet->fields is passed from outside
1702-
*/
17031699
static enum_func_status
17041700
php_mysqlnd_rowp_read(MYSQLND_CONN_DATA * conn, void * _packet)
17051701
{
@@ -1760,33 +1756,10 @@ php_mysqlnd_rowp_read(MYSQLND_CONN_DATA * conn, void * _packet)
17601756
DBG_INF_FMT("server_status=%u warning_count=%u", packet->server_status, packet->warning_count);
17611757
}
17621758
} else {
1759+
packet->eof = FALSE;
17631760
MYSQLND_INC_CONN_STATISTIC(stats,
17641761
packet->binary_protocol? STAT_ROWS_FETCHED_FROM_SERVER_PS:
17651762
STAT_ROWS_FETCHED_FROM_SERVER_NORMAL);
1766-
1767-
packet->eof = FALSE;
1768-
/* packet->field_count is set by the user of the packet */
1769-
1770-
if (!packet->skip_extraction) {
1771-
if (!packet->fields) {
1772-
DBG_INF("Allocating packet->fields");
1773-
/*
1774-
old-API will probably set packet->fields to NULL every time, though for
1775-
unbuffered sets it makes not much sense as the zvals in this buffer matter,
1776-
not the buffer. Constantly allocating and deallocating brings nothing.
1777-
1778-
For PS - if stmt_store() is performed, thus we don't have a cursor, it will
1779-
behave just like old-API buffered. Cursors will behave like a bit different,
1780-
but mostly like old-API unbuffered and thus will populate this array with
1781-
value.
1782-
*/
1783-
packet->fields = mnd_ecalloc(packet->field_count, sizeof(zval));
1784-
}
1785-
} else {
1786-
MYSQLND_INC_CONN_STATISTIC(stats,
1787-
packet->binary_protocol? STAT_ROWS_SKIPPED_PS:
1788-
STAT_ROWS_SKIPPED_NORMAL);
1789-
}
17901763
}
17911764

17921765
end:
@@ -1807,13 +1780,6 @@ php_mysqlnd_rowp_free_mem(void * _packet)
18071780
p->result_set_memory_pool->free_chunk(p->result_set_memory_pool, p->row_buffer.ptr);
18081781
p->row_buffer.ptr = NULL;
18091782
}
1810-
/*
1811-
Don't free packet->fields :
1812-
- normal queries -> store_result() | fetch_row_unbuffered() will transfer
1813-
the ownership and NULL it.
1814-
- PS will pass in it the bound variables, we have to use them! and of course
1815-
not free the array. As it is passed to us, we should not clean it ourselves.
1816-
*/
18171783
DBG_VOID_RETURN;
18181784
}
18191785
/* }}} */

‎ext/mysqlnd/mysqlnd_wireprotocol.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,6 @@ typedef struct st_mysqlnd_packet_res_field {
206206
/* Row packet */
207207
typedef struct st_mysqlnd_packet_row {
208208
MYSQLND_PACKET_HEADER header;
209-
zval *fields;
210209
uint32_t field_count;
211210
zend_bool eof;
212211
/*
@@ -219,7 +218,6 @@ typedef struct st_mysqlnd_packet_row {
219218
MYSQLND_ROW_BUFFER row_buffer;
220219
MYSQLND_MEMORY_POOL * result_set_memory_pool;
221220

222-
zend_bool skip_extraction;
223221
zend_bool binary_protocol;
224222
MYSQLND_FIELD *fields_metadata;
225223

0 commit comments

Comments
 (0)