Merge branch 'notEvil-adjust_string'

espruino · Feb 27, 2023 · 95e8a32 · 95e8a32
2 parents a74f5cb + 6b12051
commit 95e8a32
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 24 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -54,6 +54,7 @@
             Bangle.js2: Fix Compass Z heading (fix #2332)
             Micro:bit 1: remove support for paletted images to free up flash
             Bangle.js: Fix for Bangle.is* functions after #2323 fix
+            MakeIntoVariableName now shifts oversize strings around rather than re-allocating, which reduces fragmentation (#2329)
 
      2v16 : JIT functions now execute in their own function scope (allows arguments)
             Move older 'HY' boards to use `g` for the built-in graphics, not `LCD` (and change docs)

diff --git a/src/jsvar.c b/src/jsvar.c
@@ -1213,41 +1213,60 @@ JsVar *jsvMakeIntoVariableName(JsVar *var, JsVar *valueOrZero) {
     var->flags = (JsVarFlags)(var->flags & ~JSV_VARTYPEMASK) | t;
   } else if (varType>=_JSV_STRING_START && varType<=_JSV_STRING_END) {
     if (jsvGetCharactersInVar(var) > JSVAR_DATA_STRING_NAME_LEN) {
-      /* Argh. String is too large to fit in a JSV_NAME! We must make
+      /* Argh. String is too large to fit in a JSV_NAME! We must chomp
        * new STRINGEXTs to put the data in
        */
       JsvStringIterator it;
+      char queue[JSVAR_DATA_STRING_LEN - JSVAR_DATA_STRING_NAME_LEN];
+      int index;
+
+      // fill queue
       jsvStringIteratorNew(&it, var, JSVAR_DATA_STRING_NAME_LEN);
-      JsVar *startExt = jsvNewWithFlags(JSV_STRING_EXT_0);
-      JsVar *ext = jsvLockAgainSafe(startExt);
-      size_t nChars = 0;
-      while (ext && jsvStringIteratorHasChar(&it)) {
-        if (nChars >= JSVAR_DATA_STRING_MAX_LEN) {
-          jsvSetCharactersInVar(ext, nChars);
-          JsVar *ext2 = jsvNewWithFlags(JSV_STRING_EXT_0);
-          if (ext2) {
-            jsvSetLastChild(ext, jsvGetRef(ext2));
-          }
-          jsvUnLock(ext);
-          ext = ext2;
-          nChars = 0;
-        }
-        ext->varData.str[nChars++] = jsvStringIteratorGetCharAndNext(&it);
+      for (index = 0; index < sizeof(queue) && jsvStringIteratorHasChar(&it); index++) {
+        queue[index] = jsvStringIteratorGetCharAndNext(&it);
       }
       jsvStringIteratorFree(&it);
-      if (ext) {
-        jsvSetCharactersInVar(ext, nChars);
-        jsvUnLock(ext);
+
+      // "set" string length
+      JsVar* last = var;
+      while (jsvGetLastChild(last)) last = jsvGetAddressOf(jsvGetLastChild(last)); // TODO lock?
+
+      if (last != var) {
+        size_t nChars = jsvGetCharactersInVar(last) + index;
+        if (nChars <= JSVAR_DATA_STRING_MAX_LEN) { // fit inside existing StringExt
+          jsvSetCharactersInVar(last, nChars);
+          last = 0;
+        } else {
+          index = nChars - JSVAR_DATA_STRING_MAX_LEN; // remaining
+        }
       }
+      if (last) { // no StringExt or didn't fit
+        jsvSetCharactersInVar(last, jsvGetMaxCharactersInVar(last));
+        JsVar* ext = jsvNewWithFlags(JSV_STRING_EXT_0);
+        if (ext) {
+          jsvSetCharactersInVar(ext, index);
+          jsvSetLastChild(last, jsvGetRef(ext));
+          jsvUnLock(ext);
+        } // TODO else?
+      }
+
+      // juggle characters
+      jsvStringIteratorNew(&it, var, JSVAR_DATA_STRING_LEN);
+      assert(it.var == jsvGetAddressOf(jsvGetLastChild(var)) && it.charIdx == 0);
+      index = 0;
+      while (jsvStringIteratorHasChar(&it)) {
+        char c = jsvStringIteratorGetChar(&it);
+        jsvStringIteratorSetChar(&it, queue[index]);
+        queue[index] = c;
+
+        jsvStringIteratorNext(&it);
+        index = (index + 1) % sizeof(queue);
+      }
+      jsvStringIteratorFree(&it);
       jsvSetCharactersInVar(var, JSVAR_DATA_STRING_NAME_LEN);
-      // Free any old stringexts
-      jsvFreePtrStringExt(var);
-      // set up new stringexts
-      jsvSetLastChild(var, jsvGetRef(startExt));
       jsvSetNextSibling(var, 0);
       jsvSetPrevSibling(var, 0);
       jsvSetFirstChild(var, 0);
-      jsvUnLock(startExt);
     }
 
     size_t t = JSV_NAME_STRING_0;

diff --git a/tests/test_long_string_to_name.js b/tests/test_long_string_to_name.js
@@ -0,0 +1,26 @@
+o = {};
+n = 'a';
+i = 1;
+while (true) {
+  o[n] = i++;
+  n += (i % 10) || (i / 10);
+  if (E.getSizeOf(n) == 3) break;
+}
+/* o.a == 1
+ * o.a2 == 2
+ * ..
+ * o.a234567891 == 10
+ * o.a2345678911 == 11
+ * ..
+ */
+while (true) {
+  n = n.substring(0, n.length - 1);
+  if (!n.length) {
+    result = true;
+    break;
+  }
+  if (o[n] != n.length) {
+    result = false;
+    break;
+  }
+}