Merge pull request #72 from TysonAndre/string-reuse

crazyxman · web-flow · commit f0dac4ee914e · 2022-10-09T11:33:08.000+08:00
2.0.5: Reuse PHP's interned 1-character and empty strings
diff --git a/benchmark/composer.json b/benchmark/composer.json
@@ -6,10 +6,10 @@
     "minimum-stability": "dev",
     "prefer-stable": true,
     "require": {
-        "php": "^7.1"
+        "php": "^7.1|^8.2"
     },
     "require-dev": {
-        "phpbench/phpbench": "^0.17.1"
+        "phpbench/phpbench": "^1.2.6"
     },
     "autoload-dev": {
         "psr-4": {
diff --git a/benchmark/phpbench/SingleCharStringsBench.php b/benchmark/phpbench/SingleCharStringsBench.php
@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SimdjsonBench;
+
+use PhpBench\Benchmark\Metadata\Annotations\Subject;
+
+if (!extension_loaded('simdjson')) {
+        exit;
+}
+
+/**
+ * This tests decoding an array with 100 objects where keys are single-char strings and empty strings.
+ *
+ * In simdjson, those are optimized for memory usage in php 7.2+ by using php's interned string constants instead of brand new strings.
+ * (json_decode doesn't do that but could - when using the simdjson C module it's quicker since the length is known before we need to allocate memory for the internal PHP string representation (zend_string))
+ *
+ * Note that in this benchmark, PHP garbage collecting the array returned by simdjson_decode can skip over the array keys,
+ * because none of the keys were reference counted.
+ *
+ * @Revs(5)
+ * @Iterations(5)
+ * @Warmup(3)
+ * @OutputTimeUnit("milliseconds", precision=5)
+ * @BeforeMethods({"init"})
+ * @Groups({"decode"})
+ */
+class SingleCharStringsBench
+{
+
+    /**
+     * @var string
+     */
+    private $json;
+
+    public function init(): void
+    {
+        $raw = [];
+        for ($i = 0; $i < 100; $i++) {
+            $raw[] = ['x' => chr(48 + $i % 10), 'y' => chr(48 + $i % 8), 's' => ''];
+        }
+        $this->json = \json_encode($raw);
+    }
+
+    /**
+     * @Subject()
+     */
+    public function jsonDecodeAssoc(): void
+    {
+        $data = \json_decode($this->json, true);
+
+        if ('0' !== $data[0]['x']) {
+            throw new \RuntimeException('error');
+        }
+    }
+
+    /**
+     * @Subject()
+     */
+    public function jsonDecode(): void
+    {
+        $data = \json_decode($this->json, false);
+
+        if ('0' !== $data[0]->x) {
+            throw new \RuntimeException('error');
+        }
+    }
+
+    /**
+     * @Subject()
+     */
+    public function simdjsonDecodeAssoc()
+    {
+        $data = \simdjson_decode($this->json, true);
+
+        if ('0' !== $data[0]['x']) {
+            throw new \RuntimeException('error');
+        }
+    }
+
+    /**
+     * @Subject()
+     */
+    public function simdjsonDecode()
+    {
+        $data = \simdjson_decode($this->json, false);
+
+        if ('0' !== $data[0]->x) {
+            throw new \RuntimeException('error');
+        }
+    }
+
+}
diff --git a/package.xml b/package.xml
@@ -20,19 +20,16 @@
  -->
  <date>2022-10-01</date>
  <version>
-  <release>2.0.4</release>
-  <api>2.0.4</api>
+  <release>2.0.5</release>
+  <api>2.0.5</api>
  </version>
  <stability>
   <release>stable</release>
   <api>stable</api>
  </stability>
  <license uri="https://www.apache.org/licenses/LICENSE-2.0.html">Apache 2.0</license>
  <notes>
-* Add `-fvisibility=hidden` to compiler options to reduce compiled extension size by avoiding exporting symbols by default.
-* If the requested json parsing $depth is excessively large when reallocating larger buffers for the C simdjson parser,
-  then internally use a smaller $depth that would behave identically with lower memory usage. (#66)
-* Update simdjson to properly reject surrogate pairs with an invalid low surrogate. (https://en.wikipedia.org/wiki/UTF-16)
+* Reuse PHP's 1-byte and 0-byte interned strings in simdjson_decode, reducing memory usage for those strings. (e.g. for the key/value in '{"x":""}')
  </notes>
  <contents>
   <dir name="/">
@@ -59,6 +56,7 @@
     <file name="decode_invalid_property.phpt" role="test"/>
     <file name="decode_max_depth.phpt" role="test"/>
     <file name="decode_max_depth_memory_reduction.phpt" role="test"/>
+    <file name="decode_repeat.phpt" role="test"/>
     <file name="decode_result.phpt" role="test"/>
     <file name="decode_strict_types.phpt" role="test"/>
     <file name="decode_types.phpt" role="test"/>
@@ -93,6 +91,23 @@
  <providesextension>simdjson</providesextension>
  <extsrcrelease/>
  <changelog>
+   <date>2022-10-01</date>
+   <version>
+     <release>2.0.4</release>
+     <api>2.0.4</api>
+   </version>
+   <stability>
+     <release>stable</release>
+     <api>stable</api>
+   </stability>
+   <license uri="https://www.apache.org/licenses/LICENSE-2.0.html">Apache 2.0</license>
+   <notes>
+* Add `-fvisibility=hidden` to compiler options to reduce compiled extension size by avoiding exporting symbols by default.
+* If the requested json parsing $depth is excessively large when reallocating larger buffers for the C simdjson parser,
+  then internally use a smaller $depth that would behave identically with lower memory usage. (#66)
+* Update simdjson to fix handling of surrogate pairs with invalid low surrogate.
+   </notes>
+  </release>
   <release>
    <date>2022-08-30</date>
    <version>
diff --git a/php_simdjson.h b/php_simdjson.h
@@ -17,7 +17,7 @@
 extern zend_module_entry simdjson_module_entry;
 #define phpext_simdjson_ptr &simdjson_module_entry
 
-#define PHP_SIMDJSON_VERSION                  "2.0.4"
+#define PHP_SIMDJSON_VERSION                  "2.0.5"
 #define SIMDJSON_SUPPORT_URL                  "https://github.com/crazyxman/simdjson_php"
 #define SIMDJSON_PARSE_FAIL                   0
 #define SIMDJSON_PARSE_SUCCESS                1
@@ -26,9 +26,6 @@ extern zend_module_entry simdjson_module_entry;
 
 #define SIMDJSON_PARSE_DEFAULT_DEPTH          512
 
-#define SIMDJSON_RESOUCE_PJH_TYPE             3
-#define SIMDJSON_RESOUCE_PJ_TYPE              4
-
 
 extern PHPAPI void php_var_dump(zval **struc, int level);
 extern PHPAPI void php_debug_zval_dump(zval **struc, int level);
diff --git a/src/bindings.cpp b/src/bindings.cpp
@@ -71,7 +71,46 @@ build_parsed_json_cust(simdjson::dom::parser& parser, simdjson::dom::element &do
     return simdjson::SUCCESS;
 }
 
-/* }}} */
+static zend_always_inline void simdjson_set_zval_to_string(zval *v, const char *buf, size_t len) {
+    /* In php 7.1, the ZSTR_CHAR macro doesn't exist, and CG(one_char_string)[chr] may or may not be null */
+#if PHP_VERSION_ID >= 70200
+    if (len <= 1) {
+        /*
+        A note on performance benefits of the use of interned strings here and elsewhere:
+
+        - PHP doesn't need to allocate a temporary string and initialize it
+        - PHP doesn't need to free the temporary string
+        - PHP doesn't need to compute the hash of the temporary string
+        - Memory usage is reduced because the string representation is reused
+        - String comparisons are faster when the strings are the exact same pointer.
+        - CPU caches may already have this interned string
+        - If all array keys are interned strings, then php can skip the step of
+          freeing array keys when garbage collecting the array.
+         */
+        zend_string *key = len == 1 ? ZSTR_CHAR(buf[0]) : ZSTR_EMPTY_ALLOC();
+        ZVAL_INTERNED_STR(v, key);
+        return;
+    }
+#endif
+    ZVAL_STRINGL(v, buf, len);
+}
+
+static zend_always_inline void simdjson_add_key_to_symtable(HashTable *ht, const char *buf, size_t len, zval *value) {
+#if PHP_VERSION_ID >= 70200
+    if (len <= 1) {
+        /* Look up the interned string (i.e. not reference counted) */
+        zend_string *key = len == 1 ? ZSTR_CHAR(buf[0]) : ZSTR_EMPTY_ALLOC();
+        /* Add the key or update the existing value of the key. */
+        zend_symtable_update(ht, key, value);
+        /* zend_string_release_ex is a no-op for interned strings */
+        return;
+    }
+#endif
+    zend_string *key = zend_string_init(buf, len, 0);
+    zend_symtable_update(ht, key, value);
+    /* Release the reference counted key */
+    zend_string_release_ex(key, 0);
+}
 
 static zend_always_inline void simdjson_set_zval_to_int64(zval *zv, const int64_t value) {
 #if SIZEOF_ZEND_LONG < 8
@@ -89,7 +128,7 @@ static zval create_array(simdjson::dom::element element) /* {{{ */ {
     switch (element.type()) {
         //ASCII sort
         case simdjson::dom::element_type::STRING :
-            ZVAL_STRINGL(&v, element.get_c_str().value_unsafe(), element.get_string_length().value_unsafe());
+            simdjson_set_zval_to_string(&v, element.get_c_str().value_unsafe(), element.get_string_length().value_unsafe());
             break;
         case simdjson::dom::element_type::INT64 :
             simdjson_set_zval_to_int64(&v, element.get_int64().value_unsafe());
@@ -141,9 +180,7 @@ static zval create_array(simdjson::dom::element element) /* {{{ */ {
             for (simdjson::dom::key_value_pair field : json_object) {
                 zval value = create_array(field.value);
                 /* TODO consider using zend_string_init_existing_interned in php 8.1+ to save memory and time freeing strings. */
-                zend_string *key = zend_string_init(field.key.data(), field.key.size(), 0);
-                zend_symtable_update(arr, key, &value);
-                zend_string_release_ex(key, 0);
+                simdjson_add_key_to_symtable(arr, field.key.data(), field.key.size(), &value);
             }
             break;
         }
@@ -161,7 +198,7 @@ static zval create_object(simdjson::dom::element element) /* {{{ */ {
     switch (element.type()) {
         //ASCII sort
         case simdjson::dom::element_type::STRING :
-            ZVAL_STRINGL(&v, element.get_c_str().value_unsafe(), element.get_string_length().value_unsafe());
+            simdjson_set_zval_to_string(&v, element.get_c_str().value_unsafe(), element.get_string_length().value_unsafe());
             break;
         case simdjson::dom::element_type::INT64 :
             simdjson_set_zval_to_int64(&v, element.get_int64().value_unsafe());
@@ -214,14 +251,36 @@ static zval create_object(simdjson::dom::element element) /* {{{ */ {
                     return v;
                 }
                 zval value = create_object(field.value);
+
+                /* Add the key to the object */
 #if PHP_VERSION_ID >= 80000
-                /* TODO consider using zend_string_init_existing_interned in php 8.1+ to save memory and time freeing strings. */
-                zend_string *key = zend_string_init(data, size, 0);
-                obj->handlers->write_property(obj, key, &value, NULL);
+                zend_string *key;
+                if (size <= 1) {
+                    key = size == 1 ? ZSTR_CHAR(data[0]) : ZSTR_EMPTY_ALLOC();
+                } else {
+                    key = zend_string_init(data, size, 0);
+                }
+                zend_std_write_property(obj, key, &value, NULL);
                 zend_string_release_ex(key, 0);
 #else
-                add_property_zval_ex(&v, data, size, &value);
+
+# if PHP_VERSION_ID >= 70200
+                if (size <= 1) {
+                    zval zkey;
+                    zend_string *key = size == 1 ? ZSTR_CHAR(data[0]) : ZSTR_EMPTY_ALLOC();
+                    ZVAL_INTERNED_STR(&zkey, key);
+                    zend_std_write_property(&v, &zkey, &value, NULL);
+                } else
+# endif
+                {
+                    zval zkey;
+                    ZVAL_STRINGL(&zkey, data, size);
+                    zend_std_write_property(&v, &zkey, &value, NULL);
+                    zval_ptr_dtor_nogc(&zkey);
+                }
 #endif
+                /* After the key is added to the object (incrementing the reference count) ,
+                 * decrement the reference count of the value by one */
                 zval_ptr_dtor_nogc(&value);
             }
             break;
diff --git a/tests/decode_repeat.phpt b/tests/decode_repeat.phpt
@@ -0,0 +1,35 @@
+--TEST--
+simdjson_decode repeated keys use the last value
+--FILE--
+<?php
+// Repeated
+$json = '{"a":"b","a":"c","0":[1],"0":[2],"foo":"bar","foo":"baz"}';
+$value = \simdjson_decode($json, false);
+var_dump($value);
+
+$value = \simdjson_decode($json, true);
+var_dump($value);
+?>
+--EXPECT--
+object(stdClass)#1 (3) {
+  ["a"]=>
+  string(1) "c"
+  ["0"]=>
+  array(1) {
+    [0]=>
+    int(2)
+  }
+  ["foo"]=>
+  string(3) "baz"
+}
+array(3) {
+  ["a"]=>
+  string(1) "c"
+  [0]=>
+  array(1) {
+    [0]=>
+    int(2)
+  }
+  ["foo"]=>
+  string(3) "baz"
+}
