From e8be7137ca56091ef5969d780eb6600d3a68c6f0 Mon Sep 17 00:00:00 2001 From: "nboyd%atg.com" Date: Sat, 29 Sep 2001 20:55:36 +0000 Subject: [PATCH] Patch from Igor: As profiler data show, the execution time of the nextNode and replaceCurrent methods in PreorderNodeIterator contribute quite significantly to the total time to run Context.compileReader. replaceCurrent is slow because it calls Node.replaceChild which have to iterate through all previous siblings to find the nearest to the current. But it is easy to avoid this search by caching the previous sibling of the current while iterating over the node tree in nextNode. nextNode slowness is attributed to the usage of java.lang.Stack which is expensive due to its synchronized methods. In the attched patch I replaced it by the explicit array management. It allows to cut Context.compileReader time by 5%-30% when processing 20K-3MB sources assembled form JS files in the test suite. git-svn-id: svn://10.0.0.236/trunk@104228 18797224-902f-48f8-a5cc-f745e15eee43 --- .../src/org/mozilla/javascript/Node.java | 9 +++ .../javascript/PreorderNodeIterator.java | 79 +++++++++++++------ 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/Node.java b/mozilla/js/rhino/src/org/mozilla/javascript/Node.java index acd06cceabc..888b925573e 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/Node.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/Node.java @@ -254,6 +254,15 @@ public class Node implements Cloneable { child.next = null; } + public void replaceChildAfter(Node prevChild, Node newChild) { + Node child = prevChild.next; + newChild.next = child.next; + prevChild.next = newChild; + if (child == last) + last = newChild; + child.next = null; + } + public static final int TARGET_PROP = 1, BREAK_PROP = 2, diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/PreorderNodeIterator.java b/mozilla/js/rhino/src/org/mozilla/javascript/PreorderNodeIterator.java index 6a9f3369bbb..3a3015f9351 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/PreorderNodeIterator.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/PreorderNodeIterator.java @@ -21,6 +21,7 @@ * Contributor(s): * Norris Boyd * Roger Lawrence + * Igor Bukanov * * Alternatively, the contents of this file may be used under the * terms of the GNU Public License (the "GPL"), in which case the @@ -36,8 +37,6 @@ package org.mozilla.javascript; -import java.util.Stack; - /** * This class implements a preorder tree iterator for the Node class. * @@ -46,8 +45,7 @@ import java.util.Stack; */ public class PreorderNodeIterator { public PreorderNodeIterator(Node n) { - start = n; - stack = new Stack(); + start = n; } public Node currentNode() { @@ -55,38 +53,69 @@ public class PreorderNodeIterator { } public Node getCurrentParent() { - return currentParent; + return currentParent; } public Node nextNode() { - if (current == null) - return current = start; - if (current.first != null) { - stack.push(current); - currentParent = current; - current = current.first; - } else { - current = current.next; - boolean isEmpty; - for (;;) { - isEmpty = stack.isEmpty(); - if (isEmpty || current != null) - break; - current = (Node) stack.pop(); + if (current == null) { + current = start; + } + else if (current.first != null) { + stackPush(current); + currentParent = current; + cachedPrev = null; + current = current.first; + } + else { + cachedPrev = current; + current = current.next; + while (current == null && stackTop != 0) { + --stackTop; + current = stack[stackTop]; + stack[stackTop] = null; + + cachedPrev = current; current = current.next; - } - currentParent = isEmpty ? null : (Node) stack.peek(); - } - return current; + } + currentParent = (current == null) ? null : stack[stackTop - 1]; + } + return current; } public void replaceCurrent(Node newNode) { - currentParent.replaceChild(current, newNode); + if (cachedPrev != null && cachedPrev.next == current) { + // Check cachedPrev.next == current is necessary due to possible + // tree mutations + currentParent.replaceChildAfter(cachedPrev, newNode); + } + else { + currentParent.replaceChild(current, newNode); + } current = newNode; } + + private void stackPush(Node n) { + int N = stackTop; + if (N == 0) { + stack = new Node[16]; + } + else if (N == stack.length) { + Node[] tmp = new Node[N * 2]; + System.arraycopy(stack, 0, tmp, 0, N); + stack = tmp; + } + stack[N] = n; + stackTop = N + 1; + } private Node start; + private Node[] stack; + private int stackTop; + private Node current; private Node currentParent; - private Stack stack; + +//cache previous sibling of current not to search for it when +//replacing current + private Node cachedPrev; }