Skip to content

Commit c6655fb

Browse files
ndosschendossche
committed
Implement dom_get_doc_props_read_only()
I was surprised to see that getting the stricterror property showed in in the Callgrind profile of some tests. Turns out we sometimes allocate them. Fix this by returning the default in case no changes were made yet. Closes GH-11345.
1 parent d8102e6 commit c6655fb

File tree

4 files changed

+43
-43
lines changed

4 files changed

+43
-43
lines changed

‎UPGRADING.INTERNALS‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ PHP 8.3 INTERNALS UPGRADE NOTES
116116
- The PHPAPI spl_iterator_apply() function now returns zend_result instead of int.
117117
There are no functional changes.
118118

119+
f. ext/dom
120+
- A new function dom_get_doc_props_read_only() is added to gather the document
121+
properties in a read-only way. This function avoids allocation when there are
122+
no document properties changed yet.
123+
119124
========================
120125
4. OpCode changes
121126
========================

‎ext/dom/document.c‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ readonly=no
297297
int dom_document_format_output_read(dom_object *obj, zval *retval)
298298
{
299299
if (obj->document) {
300-
dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document);
300+
libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document);
301301
ZVAL_BOOL(retval, doc_prop->formatoutput);
302302
} else {
303303
ZVAL_FALSE(retval);
@@ -322,7 +322,7 @@ readonly=no
322322
int dom_document_validate_on_parse_read(dom_object *obj, zval *retval)
323323
{
324324
if (obj->document) {
325-
dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document);
325+
libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document);
326326
ZVAL_BOOL(retval, doc_prop->validateonparse);
327327
} else {
328328
ZVAL_FALSE(retval);
@@ -347,7 +347,7 @@ readonly=no
347347
int dom_document_resolve_externals_read(dom_object *obj, zval *retval)
348348
{
349349
if (obj->document) {
350-
dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document);
350+
libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document);
351351
ZVAL_BOOL(retval, doc_prop->resolveexternals);
352352
} else {
353353
ZVAL_FALSE(retval);
@@ -372,7 +372,7 @@ readonly=no
372372
int dom_document_preserve_whitespace_read(dom_object *obj, zval *retval)
373373
{
374374
if (obj->document) {
375-
dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document);
375+
libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document);
376376
ZVAL_BOOL(retval, doc_prop->preservewhitespace);
377377
} else {
378378
ZVAL_FALSE(retval);
@@ -397,7 +397,7 @@ readonly=no
397397
int dom_document_recover_read(dom_object *obj, zval *retval)
398398
{
399399
if (obj->document) {
400-
dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document);
400+
libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document);
401401
ZVAL_BOOL(retval, doc_prop->recover);
402402
} else {
403403
ZVAL_FALSE(retval);
@@ -422,7 +422,7 @@ readonly=no
422422
int dom_document_substitue_entities_read(dom_object *obj, zval *retval)
423423
{
424424
if (obj->document) {
425-
dom_doc_propsptr doc_prop = dom_get_doc_props(obj->document);
425+
libxml_doc_props const* doc_prop = dom_get_doc_props_read_only(obj->document);
426426
ZVAL_BOOL(retval, doc_prop->substituteentities);
427427
} else {
428428
ZVAL_FALSE(retval);
@@ -1176,7 +1176,6 @@ static xmlDocPtr dom_document_parser(zval *id, int mode, char *source, size_t so
11761176
{
11771177
xmlDocPtr ret;
11781178
xmlParserCtxtPtr ctxt = NULL;
1179-
dom_doc_propsptr doc_props;
11801179
dom_object *intern;
11811180
php_libxml_ref_obj *document = NULL;
11821181
int validate, recover, resolve_externals, keep_blanks, substitute_ent;
@@ -1189,17 +1188,13 @@ static xmlDocPtr dom_document_parser(zval *id, int mode, char *source, size_t so
11891188
document = intern->document;
11901189
}
11911190

1192-
doc_props = dom_get_doc_props(document);
1191+
libxml_doc_props const* doc_props = dom_get_doc_props_read_only(document);
11931192
validate = doc_props->validateonparse;
11941193
resolve_externals = doc_props->resolveexternals;
11951194
keep_blanks = doc_props->preservewhitespace;
11961195
substitute_ent = doc_props->substituteentities;
11971196
recover = doc_props->recover;
11981197

1199-
if (document == NULL) {
1200-
efree(doc_props);
1201-
}
1202-
12031198
xmlInitParser();
12041199

12051200
if (mode == DOM_LOAD_FILE) {
@@ -1387,7 +1382,6 @@ PHP_METHOD(DOMDocument, save)
13871382
size_t file_len = 0;
13881383
int bytes, format, saveempty = 0;
13891384
dom_object *intern;
1390-
dom_doc_propsptr doc_props;
13911385
char *file;
13921386
zend_long options = 0;
13931387

@@ -1405,7 +1399,7 @@ PHP_METHOD(DOMDocument, save)
14051399

14061400
/* encoding handled by property on doc */
14071401

1408-
doc_props = dom_get_doc_props(intern->document);
1402+
libxml_doc_props const* doc_props = dom_get_doc_props_read_only(intern->document);
14091403
format = doc_props->formatoutput;
14101404
if (options & LIBXML_SAVE_NOEMPTYTAG) {
14111405
saveempty = xmlSaveNoEmptyTags;
@@ -1433,7 +1427,6 @@ PHP_METHOD(DOMDocument, saveXML)
14331427
xmlBufferPtr buf;
14341428
xmlChar *mem;
14351429
dom_object *intern, *nodeobj;
1436-
dom_doc_propsptr doc_props;
14371430
int size, format, saveempty = 0;
14381431
zend_long options = 0;
14391432

@@ -1444,7 +1437,7 @@ PHP_METHOD(DOMDocument, saveXML)
14441437

14451438
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
14461439

1447-
doc_props = dom_get_doc_props(intern->document);
1440+
libxml_doc_props const* doc_props = dom_get_doc_props_read_only(intern->document);
14481441
format = doc_props->formatoutput;
14491442

14501443
if (nodep != NULL) {
@@ -1928,7 +1921,6 @@ PHP_METHOD(DOMDocument, saveHTMLFile)
19281921
size_t file_len;
19291922
int bytes, format;
19301923
dom_object *intern;
1931-
dom_doc_propsptr doc_props;
19321924
char *file;
19331925
const char *encoding;
19341926

@@ -1947,7 +1939,7 @@ PHP_METHOD(DOMDocument, saveHTMLFile)
19471939

19481940
encoding = (const char *) htmlGetMetaEncoding(docp);
19491941

1950-
doc_props = dom_get_doc_props(intern->document);
1942+
libxml_doc_props const* doc_props = dom_get_doc_props_read_only(intern->document);
19511943
format = doc_props->formatoutput;
19521944
bytes = htmlSaveFileFormat(file, docp, encoding, format);
19531945

@@ -1969,7 +1961,6 @@ PHP_METHOD(DOMDocument, saveHTML)
19691961
dom_object *intern, *nodeobj;
19701962
xmlChar *mem = NULL;
19711963
int format;
1972-
dom_doc_propsptr doc_props;
19731964

19741965
id = ZEND_THIS;
19751966
if (zend_parse_parameters(ZEND_NUM_ARGS(),
@@ -1980,7 +1971,7 @@ PHP_METHOD(DOMDocument, saveHTML)
19801971

19811972
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
19821973

1983-
doc_props = dom_get_doc_props(intern->document);
1974+
libxml_doc_props const* doc_props = dom_get_doc_props(intern->document);
19841975
format = doc_props->formatoutput;
19851976

19861977
if (nodep != NULL) {

‎ext/dom/php_dom.c‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,17 @@ int dom_node_children_valid(xmlNodePtr node) {
140140
}
141141
/* }}} end dom_node_children_valid */
142142

143+
static const libxml_doc_props default_doc_props = {
144+
.formatoutput = false,
145+
.validateonparse = false,
146+
.resolveexternals = false,
147+
.preservewhitespace = true,
148+
.substituteentities = false,
149+
.stricterror = true,
150+
.recover = false,
151+
.classmap = NULL,
152+
};
153+
143154
/* {{{ dom_get_doc_props() */
144155
dom_doc_propsptr dom_get_doc_props(php_libxml_ref_obj *document)
145156
{
@@ -149,28 +160,31 @@ dom_doc_propsptr dom_get_doc_props(php_libxml_ref_obj *document)
149160
return document->doc_props;
150161
} else {
151162
doc_props = emalloc(sizeof(libxml_doc_props));
152-
doc_props->formatoutput = 0;
153-
doc_props->validateonparse = 0;
154-
doc_props->resolveexternals = 0;
155-
doc_props->preservewhitespace = 1;
156-
doc_props->substituteentities = 0;
157-
doc_props->stricterror = 1;
158-
doc_props->recover = 0;
159-
doc_props->classmap = NULL;
163+
memcpy(doc_props, &default_doc_props, sizeof(libxml_doc_props));
160164
if (document) {
161165
document->doc_props = doc_props;
162166
}
163167
return doc_props;
164168
}
165169
}
170+
/* }}} */
171+
172+
libxml_doc_props const* dom_get_doc_props_read_only(const php_libxml_ref_obj *document)
173+
{
174+
if (document && document->doc_props) {
175+
return document->doc_props;
176+
} else {
177+
return &default_doc_props;
178+
}
179+
}
166180

167181
static void dom_copy_doc_props(php_libxml_ref_obj *source_doc, php_libxml_ref_obj *dest_doc)
168182
{
169-
dom_doc_propsptr source, dest;
183+
dom_doc_propsptr dest;
170184

171185
if (source_doc && dest_doc) {
172186

173-
source = dom_get_doc_props(source_doc);
187+
libxml_doc_props const* source = dom_get_doc_props_read_only(source_doc);
174188
dest = dom_get_doc_props(dest_doc);
175189

176190
dest->formatoutput = source->formatoutput;
@@ -212,10 +226,8 @@ void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece
212226

213227
zend_class_entry *dom_get_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece)
214228
{
215-
dom_doc_propsptr doc_props;
216-
217229
if (document) {
218-
doc_props = dom_get_doc_props(document);
230+
libxml_doc_props const* doc_props = dom_get_doc_props_read_only(document);
219231
if (doc_props->classmap) {
220232
zend_class_entry *ce = zend_hash_find_ptr(doc_props->classmap, basece->name);
221233
if (ce) {
@@ -230,16 +242,7 @@ zend_class_entry *dom_get_doc_classmap(php_libxml_ref_obj *document, zend_class_
230242

231243
/* {{{ dom_get_strict_error() */
232244
int dom_get_strict_error(php_libxml_ref_obj *document) {
233-
int stricterror;
234-
dom_doc_propsptr doc_props;
235-
236-
doc_props = dom_get_doc_props(document);
237-
stricterror = doc_props->stricterror;
238-
if (document == NULL) {
239-
efree(doc_props);
240-
}
241-
242-
return stricterror;
245+
return dom_get_doc_props_read_only(document)->stricterror;
243246
}
244247
/* }}} */
245248

‎ext/dom/php_dom.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ typedef struct {
9797

9898
dom_object *dom_object_get_data(xmlNodePtr obj);
9999
dom_doc_propsptr dom_get_doc_props(php_libxml_ref_obj *document);
100+
libxml_doc_props const* dom_get_doc_props_read_only(const php_libxml_ref_obj *document);
100101
zend_object *dom_objects_new(zend_class_entry *class_type);
101102
zend_object *dom_nnodemap_objects_new(zend_class_entry *class_type);
102103
#ifdef LIBXML_XPATH_ENABLED

0 commit comments

Comments
 (0)