Skip to content

Commit f5e81fe

Browse files
committed
Optimize in-memory XMLWriter
We're currently using a libxml buffer, which requires copying the buffer to zend_strings every time we want to output the string. Furthermore, its use of the system allocator instead of ZendMM makes it not count towards the memory_limit and hinders performance. This patch adds a custom writer such that the strings are written to a smart_str instance, using ZendMM for improved performance, and giving the ability to not copy the string in the common case where flush has empty set to true. Closes GH-16120.
1 parent 63e0b9c commit f5e81fe

File tree

5 files changed

+85
-30
lines changed

5 files changed

+85
-30
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,7 @@ PHP NEWS
1919
. Fixed bug #49169 (SoapServer calls wrong function, although "SOAP action"
2020
header is correct). (nielsdos)
2121

22+
- XMLWriter:
23+
. Improved performance and reduce memory consumption. (nielsdos)
24+
2225
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

UPGRADING

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,6 @@ PHP 8.5 UPGRADE NOTES
9595
========================================
9696
14. Performance Improvements
9797
========================================
98+
99+
- XMLWriter:
100+
. Improved performance and reduce memory consumption.

ext/xmlwriter/php_xmlwriter.c

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "ext/standard/info.h"
2525
#include "php_xmlwriter.h"
2626
#include "php_xmlwriter_arginfo.h"
27+
#include "zend_smart_str.h"
2728

2829
static zend_class_entry *xmlwriter_class_entry_ce;
2930

@@ -47,11 +48,9 @@ static zend_object_handlers xmlwriter_object_handlers;
4748
static zend_always_inline void xmlwriter_destroy_libxml_objects(ze_xmlwriter_object *intern)
4849
{
4950
if (intern->ptr) {
51+
/* Note: this call will also free the output pointer. */
5052
xmlFreeTextWriter(intern->ptr);
5153
intern->ptr = NULL;
52-
}
53-
if (intern->output) {
54-
xmlBufferFree(intern->output);
5554
intern->output = NULL;
5655
}
5756
}
@@ -178,14 +177,14 @@ static char *_xmlwriter_get_valid_file_path(char *source, char *resolved_path, i
178177
}
179178
/* }}} */
180179

181-
static void xml_writer_create_static(INTERNAL_FUNCTION_PARAMETERS, xmlTextWriterPtr writer, xmlBufferPtr output)
180+
static void xml_writer_create_static(INTERNAL_FUNCTION_PARAMETERS, xmlTextWriterPtr writer, smart_str *output)
182181
{
183182
if (object_init_with_constructor(return_value, Z_CE_P(ZEND_THIS), 0, NULL, NULL) == SUCCESS) {
184183
ze_xmlwriter_object *intern = Z_XMLWRITER_P(return_value);
185184
intern->ptr = writer;
186185
intern->output = output;
187186
} else {
188-
xmlBufferFree(output);
187+
// output is freed by writer, so we don't need to free it here.
189188
xmlFreeTextWriter(writer);
190189
}
191190
}
@@ -877,11 +876,45 @@ PHP_METHOD(XMLWriter, toUri)
877876
xml_writer_create_static(INTERNAL_FUNCTION_PARAM_PASSTHRU, writer, NULL);
878877
}
879878

879+
static int xml_writer_stream_write_memory(void *context, const char *buffer, int len)
880+
{
881+
smart_str *output = context;
882+
smart_str_appendl(output, buffer, len);
883+
return len;
884+
}
885+
886+
static int xml_writer_stream_close_memory(void *context)
887+
{
888+
smart_str *output = context;
889+
smart_str_free_ex(output, false);
890+
efree(output);
891+
return 0;
892+
}
893+
894+
static xmlTextWriterPtr xml_writer_create_in_memory(smart_str **output_ptr)
895+
{
896+
smart_str *output = emalloc(sizeof(*output));
897+
memset(output, 0, sizeof(*output));
898+
899+
xmlOutputBufferPtr output_buffer = xmlOutputBufferCreateIO(xml_writer_stream_write_memory, xml_writer_stream_close_memory, output, NULL);
900+
if (output_buffer == NULL) {
901+
efree(output);
902+
return NULL;
903+
}
904+
905+
xmlTextWriterPtr writer = xmlNewTextWriter(output_buffer);
906+
if (!writer) {
907+
/* This call will free output too. */
908+
xmlOutputBufferClose(output_buffer);
909+
return NULL;
910+
}
911+
*output_ptr = output;
912+
return writer;
913+
}
914+
880915
/* {{{ Create new xmlwriter using memory for string output */
881916
PHP_FUNCTION(xmlwriter_open_memory)
882917
{
883-
xmlTextWriterPtr ptr;
884-
xmlBufferPtr buffer;
885918
zval *self = getThis();
886919
ze_xmlwriter_object *ze_obj = NULL;
887920

@@ -894,28 +927,21 @@ PHP_FUNCTION(xmlwriter_open_memory)
894927
ze_obj = Z_XMLWRITER_P(self);
895928
}
896929

897-
buffer = xmlBufferCreate();
898-
899-
if (buffer == NULL) {
900-
php_error_docref(NULL, E_WARNING, "Unable to create output buffer");
901-
RETURN_FALSE;
902-
}
903-
904-
ptr = xmlNewTextWriterMemory(buffer, 0);
930+
smart_str *output;
931+
xmlTextWriterPtr ptr = xml_writer_create_in_memory(&output);
905932
if (! ptr) {
906-
xmlBufferFree(buffer);
907933
RETURN_FALSE;
908934
}
909935

910936
if (self) {
911937
xmlwriter_destroy_libxml_objects(ze_obj);
912938
ze_obj->ptr = ptr;
913-
ze_obj->output = buffer;
939+
ze_obj->output = output;
914940
RETURN_TRUE;
915941
} else {
916942
ze_obj = php_xmlwriter_fetch_object(xmlwriter_object_new(xmlwriter_class_entry_ce));
917943
ze_obj->ptr = ptr;
918-
ze_obj->output = buffer;
944+
ze_obj->output = output;
919945
RETURN_OBJ(&ze_obj->std);
920946
}
921947

@@ -926,17 +952,16 @@ PHP_METHOD(XMLWriter, toMemory)
926952
{
927953
ZEND_PARSE_PARAMETERS_NONE();
928954

929-
xmlBufferPtr buffer = xmlBufferCreate();
930-
xmlTextWriterPtr writer = xmlNewTextWriterMemory(buffer, 0);
955+
smart_str *output;
956+
xmlTextWriterPtr writer = xml_writer_create_in_memory(&output);
931957

932958
/* No need for an explicit buffer check as this will fail on a NULL buffer. */
933959
if (!writer) {
934-
xmlBufferFree(buffer);
935960
zend_throw_error(NULL, "Could not construct libxml writer");
936961
RETURN_THROWS();
937962
}
938963

939-
xml_writer_create_static(INTERNAL_FUNCTION_PARAM_PASSTHRU, writer, buffer);
964+
xml_writer_create_static(INTERNAL_FUNCTION_PARAM_PASSTHRU, writer, output);
940965
}
941966

942967
static int xml_writer_stream_write(void *context, const char *buffer, int len)
@@ -992,7 +1017,6 @@ PHP_METHOD(XMLWriter, toStream)
9921017
/* {{{ php_xmlwriter_flush */
9931018
static void php_xmlwriter_flush(INTERNAL_FUNCTION_PARAMETERS, int force_string) {
9941019
xmlTextWriterPtr ptr;
995-
xmlBufferPtr buffer;
9961020
bool empty = 1;
9971021
int output_bytes;
9981022
zval *self;
@@ -1002,16 +1026,18 @@ static void php_xmlwriter_flush(INTERNAL_FUNCTION_PARAMETERS, int force_string)
10021026
}
10031027
XMLWRITER_FROM_OBJECT(ptr, self);
10041028

1005-
buffer = Z_XMLWRITER_P(self)->output;
1006-
if (force_string == 1 && buffer == NULL) {
1029+
smart_str *output = Z_XMLWRITER_P(self)->output;
1030+
if (force_string == 1 && output == NULL) {
10071031
RETURN_EMPTY_STRING();
10081032
}
10091033
output_bytes = xmlTextWriterFlush(ptr);
1010-
if (buffer) {
1011-
const xmlChar *content = xmlBufferContent(buffer);
1012-
RETVAL_STRING((const char *) content);
1034+
if (output) {
10131035
if (empty) {
1014-
xmlBufferEmpty(buffer);
1036+
RETURN_STR(smart_str_extract(output));
1037+
} else if (smart_str_get_len(output) > 0) {
1038+
RETURN_NEW_STR(zend_string_dup(output->s, false));
1039+
} else {
1040+
RETURN_EMPTY_STRING();
10151041
}
10161042
} else {
10171043
RETVAL_LONG(output_bytes);

ext/xmlwriter/php_xmlwriter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ extern zend_module_entry xmlwriter_module_entry;
3535
/* Extends zend object */
3636
typedef struct _ze_xmlwriter_object {
3737
xmlTextWriterPtr ptr;
38-
xmlBufferPtr output;
38+
smart_str *output;
3939
zend_object std;
4040
} ze_xmlwriter_object;
4141

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
XMLWriter::toMemory() with combinations of empty flush and non-empty flush
3+
--EXTENSIONS--
4+
xmlwriter
5+
--FILE--
6+
<?php
7+
8+
$writer = XMLWriter::toMemory();
9+
var_dump($writer->flush(empty: false));
10+
$writer->startElement('foo');
11+
var_dump($writer->flush(empty: false));
12+
$writer->endElement();
13+
var_dump($writer->flush(empty: false));
14+
var_dump($writer->flush());
15+
var_dump($writer->flush());
16+
17+
?>
18+
--EXPECT--
19+
string(0) ""
20+
string(4) "<foo"
21+
string(6) "<foo/>"
22+
string(6) "<foo/>"
23+
string(0) ""

0 commit comments

Comments
 (0)