From ee01e3fa68186a5d4be873f4f273e26ab012f524 Mon Sep 17 00:00:00 2001 From: rbri Date: Wed, 27 Dec 2017 20:54:02 +0100 Subject: [PATCH 1/5] cleanup the code an try to make it faster --- src/org/mozilla/javascript/ConsString.java | 34 +++++++++++----------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/org/mozilla/javascript/ConsString.java b/src/org/mozilla/javascript/ConsString.java index 62cd9a8d43..00e154c7be 100644 --- a/src/org/mozilla/javascript/ConsString.java +++ b/src/org/mozilla/javascript/ConsString.java @@ -7,7 +7,7 @@ package org.mozilla.javascript; import java.io.Serializable; -import java.util.ArrayList; +import java.util.ArrayDeque; /** *

This class represents a string composed of two components, each of which @@ -38,12 +38,6 @@ public ConsString(CharSequence str1, CharSequence str2) { s2 = str2; length = str1.length() + str2.length(); depth = 1; - if (str1 instanceof ConsString) { - depth += ((ConsString)str1).depth; - } - if (str2 instanceof ConsString) { - depth += ((ConsString)str2).depth; - } } // Replace with string representation when serializing @@ -57,21 +51,28 @@ public String toString() { } private synchronized String flatten() { - if (depth > 0) { + if (depth == 1) { StringBuilder b = new StringBuilder(length); - ArrayList buffer = new ArrayList(); - buffer.add(s2); - buffer.add(s1); - while(!buffer.isEmpty()) { - CharSequence next = buffer.remove(buffer.size() - 1); + ArrayDeque deque = new ArrayDeque(); + deque.addFirst(s2); + + CharSequence next = s1; + do { if (next instanceof ConsString) { ConsString casted = (ConsString) next; - buffer.add(casted.s2); - buffer.add(casted.s1); + if (casted.depth == 0) { + b.append((String)casted.s1); + next = deque.isEmpty() ? null : deque.pollFirst(); + } else { + deque.addFirst(casted.s2); + next = casted.s1; + } } else { b.append(next); + next = deque.isEmpty() ? null : deque.pollFirst(); } - } + } while (next != null); + s1 = b.toString(); s2 = ""; depth = 0; @@ -92,5 +93,4 @@ public CharSequence subSequence(int start, int end) { String str = depth == 0 ? (String)s1 : flatten(); return str.substring(start, end); } - } From 45aa7a5f0b1d6d8565c47b3db0fc6664b4fa98ce Mon Sep 17 00:00:00 2001 From: rbri Date: Thu, 28 Dec 2017 09:39:52 +0100 Subject: [PATCH 2/5] using an arraylist seems to be faster --- src/org/mozilla/javascript/ConsString.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/org/mozilla/javascript/ConsString.java b/src/org/mozilla/javascript/ConsString.java index 00e154c7be..b552ed7def 100644 --- a/src/org/mozilla/javascript/ConsString.java +++ b/src/org/mozilla/javascript/ConsString.java @@ -7,7 +7,7 @@ package org.mozilla.javascript; import java.io.Serializable; -import java.util.ArrayDeque; +import java.util.ArrayList; /** *

This class represents a string composed of two components, each of which @@ -53,8 +53,8 @@ public String toString() { private synchronized String flatten() { if (depth == 1) { StringBuilder b = new StringBuilder(length); - ArrayDeque deque = new ArrayDeque(); - deque.addFirst(s2); + ArrayList buffer = new ArrayList(); + buffer.add(s2); CharSequence next = s1; do { @@ -62,14 +62,14 @@ private synchronized String flatten() { ConsString casted = (ConsString) next; if (casted.depth == 0) { b.append((String)casted.s1); - next = deque.isEmpty() ? null : deque.pollFirst(); + next = buffer.isEmpty() ? null : buffer.remove(buffer.size() - 1); } else { - deque.addFirst(casted.s2); + buffer.add(casted.s2); next = casted.s1; } } else { b.append(next); - next = deque.isEmpty() ? null : deque.pollFirst(); + next = buffer.isEmpty() ? null : buffer.remove(buffer.size() - 1); } } while (next != null); From 7eac7339af7d013c3cb9a19f36da562fc260d4da Mon Sep 17 00:00:00 2001 From: rbri Date: Thu, 28 Dec 2017 15:09:18 +0100 Subject: [PATCH 3/5] next try and more tests --- src/org/mozilla/javascript/ConsString.java | 54 ++++++++++--------- .../javascript/tests/ConsStringTest.java | 7 +++ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/org/mozilla/javascript/ConsString.java b/src/org/mozilla/javascript/ConsString.java index b552ed7def..24779028d5 100644 --- a/src/org/mozilla/javascript/ConsString.java +++ b/src/org/mozilla/javascript/ConsString.java @@ -29,15 +29,15 @@ public class ConsString implements CharSequence, Serializable { private static final long serialVersionUID = -8432806714471372570L; - private CharSequence s1, s2; + private CharSequence first, second; private final int length; - private int depth; + private boolean isFlat; public ConsString(CharSequence str1, CharSequence str2) { - s1 = str1; - s2 = str2; + first = str1; + second = str2; length = str1.length() + str2.length(); - depth = 1; + isFlat = false; } // Replace with string representation when serializing @@ -47,37 +47,41 @@ private Object writeReplace() { @Override public String toString() { - return depth == 0 ? (String)s1 : flatten(); + return isFlat ? (String)first : flatten(); } private synchronized String flatten() { - if (depth == 1) { - StringBuilder b = new StringBuilder(length); - ArrayList buffer = new ArrayList(); - buffer.add(s2); + if (!isFlat) { + final char[] chars = new char[length]; + int charPos = length; - CharSequence next = s1; + ArrayList stack = new ArrayList(); + stack.add(first); + + CharSequence next = second; do { if (next instanceof ConsString) { ConsString casted = (ConsString) next; - if (casted.depth == 0) { - b.append((String)casted.s1); - next = buffer.isEmpty() ? null : buffer.remove(buffer.size() - 1); + if (casted.isFlat) { + next = casted.first; } else { - buffer.add(casted.s2); - next = casted.s1; + stack.add(casted.first); + next = casted.second; + continue; } - } else { - b.append(next); - next = buffer.isEmpty() ? null : buffer.remove(buffer.size() - 1); } + + final String str = (String) next; + charPos -= str.length(); + str.getChars(0, str.length(), chars, charPos); + next = stack.isEmpty() ? null : stack.remove(stack.size() - 1); } while (next != null); - s1 = b.toString(); - s2 = ""; - depth = 0; + first = new String(chars); + second = ""; + isFlat = true; } - return (String)s1; + return (String)first; } public int length() { @@ -85,12 +89,12 @@ public int length() { } public char charAt(int index) { - String str = depth == 0 ? (String)s1 : flatten(); + String str = isFlat ? (String)first : flatten(); return str.charAt(index); } public CharSequence subSequence(int start, int end) { - String str = depth == 0 ? (String)s1 : flatten(); + String str = isFlat ? (String)first : flatten(); return str.substring(start, end); } } diff --git a/testsrc/org/mozilla/javascript/tests/ConsStringTest.java b/testsrc/org/mozilla/javascript/tests/ConsStringTest.java index 215b0c2d39..49581f7afd 100644 --- a/testsrc/org/mozilla/javascript/tests/ConsStringTest.java +++ b/testsrc/org/mozilla/javascript/tests/ConsStringTest.java @@ -4,12 +4,19 @@ import org.mozilla.javascript.ConsString; public class ConsStringTest extends TestCase { + public void testAppend() { ConsString current = new ConsString("a", "b"); current = new ConsString(current, "c"); current = new ConsString(current, "d"); assertEquals("abcd", current.toString()); + + current = new ConsString("x", new ConsString("a", "b")); + assertEquals("xab", current.toString()); + + current = new ConsString(new ConsString("a", "b"), new ConsString("c", "d")); + assertEquals("abcd", current.toString()); } public void testAppendManyStrings() { From 14abf0f437c99923aa7ede7b29f6b21cd17a461d Mon Sep 17 00:00:00 2001 From: rbri Date: Thu, 28 Dec 2017 17:54:50 +0100 Subject: [PATCH 4/5] again rename attributes; back to ArrayDeque but i can see no performance differences --- src/org/mozilla/javascript/ConsString.java | 38 +++++++++++----------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/org/mozilla/javascript/ConsString.java b/src/org/mozilla/javascript/ConsString.java index 24779028d5..253ce2a77a 100644 --- a/src/org/mozilla/javascript/ConsString.java +++ b/src/org/mozilla/javascript/ConsString.java @@ -7,7 +7,7 @@ package org.mozilla.javascript; import java.io.Serializable; -import java.util.ArrayList; +import java.util.ArrayDeque; /** *

This class represents a string composed of two components, each of which @@ -29,15 +29,15 @@ public class ConsString implements CharSequence, Serializable { private static final long serialVersionUID = -8432806714471372570L; - private CharSequence first, second; + private CharSequence left, right; private final int length; private boolean isFlat; public ConsString(CharSequence str1, CharSequence str2) { - first = str1; - second = str2; - length = str1.length() + str2.length(); - isFlat = false; + left = str1; + right = str2; + length = left.length() + right.length(); + isFlat = right.length() == 0; } // Replace with string representation when serializing @@ -47,7 +47,7 @@ private Object writeReplace() { @Override public String toString() { - return isFlat ? (String)first : flatten(); + return isFlat ? (String)left : flatten(); } private synchronized String flatten() { @@ -55,18 +55,18 @@ private synchronized String flatten() { final char[] chars = new char[length]; int charPos = length; - ArrayList stack = new ArrayList(); - stack.add(first); + ArrayDeque stack = new ArrayDeque(); + stack.addFirst(left); - CharSequence next = second; + CharSequence next = right; do { if (next instanceof ConsString) { ConsString casted = (ConsString) next; if (casted.isFlat) { - next = casted.first; + next = casted.left; } else { - stack.add(casted.first); - next = casted.second; + stack.addFirst(casted.left); + next = casted.right; continue; } } @@ -74,14 +74,14 @@ private synchronized String flatten() { final String str = (String) next; charPos -= str.length(); str.getChars(0, str.length(), chars, charPos); - next = stack.isEmpty() ? null : stack.remove(stack.size() - 1); + next = stack.isEmpty() ? null : stack.removeFirst(); } while (next != null); - first = new String(chars); - second = ""; + left = new String(chars); + right = ""; isFlat = true; } - return (String)first; + return (String)left; } public int length() { @@ -89,12 +89,12 @@ public int length() { } public char charAt(int index) { - String str = isFlat ? (String)first : flatten(); + String str = isFlat ? (String)left : flatten(); return str.charAt(index); } public CharSequence subSequence(int start, int end) { - String str = isFlat ? (String)first : flatten(); + String str = isFlat ? (String)left : flatten(); return str.substring(start, end); } } From e6ebe03d9cc191a6474cd94141db8c8ec573ac75 Mon Sep 17 00:00:00 2001 From: rbri Date: Fri, 29 Dec 2017 10:29:37 +0100 Subject: [PATCH 5/5] revert an stupid idea --- src/org/mozilla/javascript/ConsString.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/org/mozilla/javascript/ConsString.java b/src/org/mozilla/javascript/ConsString.java index 253ce2a77a..d8eb4537a3 100644 --- a/src/org/mozilla/javascript/ConsString.java +++ b/src/org/mozilla/javascript/ConsString.java @@ -37,7 +37,7 @@ public ConsString(CharSequence str1, CharSequence str2) { left = str1; right = str2; length = left.length() + right.length(); - isFlat = right.length() == 0; + isFlat = false; } // Replace with string representation when serializing