Factor out modification nr caching

Author: nielsdos

ext/dom/php_dom.h

@@ -252,14 +252,8 @@ xmlNodePtr dom_clone_node(php_dom_libxml_ns_mapper *ns_mapper, xmlNodePtr node,
 
 static zend_always_inline bool php_dom_is_cache_tag_stale_from_doc_ptr(const php_libxml_cache_tag *cache_tag, const php_libxml_ref_obj *doc_ptr)
 {
-	ZEND_ASSERT(cache_tag != NULL);
 	ZEND_ASSERT(doc_ptr != NULL);
-	/* See overflow comment in php_libxml_invalidate_node_list_cache(). */
-#if SIZEOF_SIZE_T == 8
-	return cache_tag->modification_nr != doc_ptr->cache_tag.modification_nr;
-#else
-	return cache_tag->modification_nr != doc_ptr->cache_tag.modification_nr || UNEXPECTED(doc_ptr->cache_tag.modification_nr == SIZE_MAX);
-#endif
+	return php_libxml_is_cache_tag_stale(cache_tag, &doc_ptr->cache_tag);
 }
 
 static zend_always_inline bool php_dom_is_cache_tag_stale_from_node(const php_libxml_cache_tag *cache_tag, const xmlNodePtr node)

ext/dom/token_list.c

@@ -32,10 +32,7 @@ typedef struct _dom_token_list_it {
 	zend_object_iterator it;
 	/* Store the hash position here to allow multiple (e.g. nested) iterations of the same token list. */
 	HashPosition pos;
-	/* Modification tracking: when the token list changes, we increment its counter.
-	 * When this counter no longer matches the counter of the token list, we know that it has changed and we
-	 * have to update the iteration index. */
-	size_t modification_nr;
+	php_libxml_cache_tag cache_tag;
 } dom_token_list_it;
 
 /* https://infra.spec.whatwg.org/#ascii-whitespace */
@@ -137,7 +134,7 @@ static void dom_token_list_update(dom_token_list_object *intern)
 	const xmlAttr *attr = dom_token_list_get_attr(intern);
 	HashTable *token_set = TOKEN_LIST_GET_SET(intern);
 
-	intern->modification_nr++;
+	php_libxml_invalidate_cache_tag(&intern->cache_tag);
 
 	/* 1. If the associated element does not have an associated attribute and token set is empty, then return. */
 	if (attr == NULL && zend_hash_num_elements(token_set) == 0) {
@@ -182,7 +179,7 @@ static void dom_token_list_ensure_set_up_to_date(dom_token_list_object *intern)
 
 	/* xmlStrEqual will automatically handle equality rules of NULL vs "" (etc) correctly. */
 	if (!xmlStrEqual(value, (const xmlChar *) intern->cached_string)) {
-		intern->modification_nr++;
+		php_libxml_invalidate_cache_tag(&intern->cache_tag);
 		efree(intern->cached_string);
 		HashTable *token_set = TOKEN_LIST_GET_SET(intern);
 		zend_hash_destroy(token_set);
@@ -199,7 +196,7 @@ void dom_token_list_ctor(dom_token_list_object *intern, dom_object *element_obj)
 	element_obj->document->refcount++;
 	intern->dom.document = element_obj->document;
 
-	intern->modification_nr = 0;
+	intern->cache_tag.modification_nr = 0;
 
 	ALLOC_HASHTABLE(TOKEN_LIST_GET_SET(intern));
 	HashTable *token_set = TOKEN_LIST_GET_SET(intern);
@@ -642,7 +639,7 @@ static void dom_token_list_it_get_current_key(zend_object_iterator *iter, zval *
 
 	dom_token_list_ensure_set_up_to_date(object);
 
-	if (UNEXPECTED(object->modification_nr != iterator->modification_nr)) {
+	if (UNEXPECTED(php_libxml_is_cache_tag_stale(&object->cache_tag, &iterator->cache_tag))) {
 		iter->index = 0;
 		HashPosition pos;
 		HashTable *token_set = TOKEN_LIST_GET_SET(object);
@@ -704,7 +701,7 @@ zend_object_iterator *dom_token_list_get_iterator(zend_class_entry *ce, zval *ob
 	ZVAL_OBJ_COPY(&iterator->it.data, Z_OBJ_P(object));
 
 	iterator->it.funcs = &dom_token_list_it_funcs;
-	iterator->modification_nr = intern->modification_nr;
+	iterator->cache_tag = intern->cache_tag;
 
 	return &iterator->it;
 }

ext/dom/token_list.h

@@ -21,8 +21,7 @@ typedef struct _dom_token_list_object {
 	php_libxml_node_ptr *element_ptr;
 	/* Used to check if the token set is up to date. */
 	char *cached_string;
-	/* See dom_token_list_it */
-	size_t modification_nr;
+	php_libxml_cache_tag cache_tag;
 	dom_object dom;
 } dom_token_list_object;
 

ext/libxml/php_libxml.h

@@ -58,6 +58,9 @@ typedef struct _libxml_doc_props {
 	bool recover;
 } libxml_doc_props;
 
+/* Modification tracking: when the object changes, we increment its counter.
+ * When this counter no longer matches the counter at the time of caching,
+ * we know that the object has changed and we have to update the cache. */
 typedef struct {
 	size_t modification_nr;
 } php_libxml_cache_tag;
@@ -109,23 +112,39 @@ static inline php_libxml_node_object *php_libxml_node_fetch_object(zend_object *
 	return (php_libxml_node_object *)((char*)(obj) - obj->handlers->offset);
 }
 
-static zend_always_inline void php_libxml_invalidate_node_list_cache(php_libxml_ref_obj *doc_ptr)
+static zend_always_inline void php_libxml_invalidate_cache_tag(php_libxml_cache_tag *cache_tag)
 {
-	if (!doc_ptr) {
-		return;
-	}
 #if SIZEOF_SIZE_T == 8
 	/* If one operation happens every nanosecond, then it would still require 584 years to overflow
 	 * the counter. So we'll just assume this never happens. */
-	doc_ptr->cache_tag.modification_nr++;
+	cache_tag->modification_nr++;
 #else
-	size_t new_modification_nr = doc_ptr->cache_tag.modification_nr + 1;
+	size_t new_modification_nr = cache_tag->modification_nr + 1;
 	if (EXPECTED(new_modification_nr > 0)) { /* unsigned overflow; checking after addition results in one less instruction */
-		doc_ptr->cache_tag.modification_nr = new_modification_nr;
+		cache_tag->modification_nr = new_modification_nr;
 	}
 #endif
 }
 
+static zend_always_inline bool php_libxml_is_cache_tag_stale(const php_libxml_cache_tag *object_tag, const php_libxml_cache_tag *cache_tag)
+{
+	ZEND_ASSERT(object_tag != NULL);
+	ZEND_ASSERT(cache_tag != NULL);
+	/* See overflow comment in php_libxml_invalidate_node_list_cache(). */
+#if SIZEOF_SIZE_T == 8
+	return cache_tag->modification_nr != object_tag->modification_nr;
+#else
+	return cache_tag->modification_nr != object_tag->modification_nr || UNEXPECTED(object_tag->modification_nr == SIZE_MAX);
+#endif
+}
+
+static zend_always_inline void php_libxml_invalidate_node_list_cache(php_libxml_ref_obj *doc_ptr)
+{
+	if (doc_ptr) {
+		php_libxml_invalidate_cache_tag(&doc_ptr->cache_tag);
+	}
+}
+
 static zend_always_inline void php_libxml_invalidate_node_list_cache_from_doc(xmlDocPtr docp)
 {
 	if (docp && docp->_private) { /* docp is NULL for detached nodes */