|
osdir.com mailing list archive |
|
Subject: [HtmlUnit] [ htmlunit-Patches-1034897 ] Fix for performance problem in JavaScript handling - msg#00030List: java.htmlunit.develMessage generated for change (Comment added) made by mbowler You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=448268&aid=1034897&group_id=47038 Category: None Group: None >Status: Closed >Resolution: Accepted Priority: 5 Submitted By: Darrell DeBoer (bigdaz) >Assigned to: Mike Bowler (mbowler) Summary: Fix for performance problem in JavaScript handling Initial Comment: We were having big performance problems using HTMLUnit to test our app, using the latest HTMLUnit from CVS. Some javascript actions would take tens of seconds to execute. Using an older version didn't give us the same trouble. I tracked down the problem, which was introduced when code to handle the "window.myVariable" style of variable access was added. The problem was caused by _all_ javascript variable and function access being routed through the Window.get() method, which then created, registered and executed a new javascript function to access the variable. For example, executing "var myDate = new Date();" would cause HtmlUnit to compile a special "GargoyleWrapper" function to access the built-in Date function. Thus a reasonably simple page could result in thousands of unnecessary round trips to the JavaScriptEngine. Anyway, I've now constructed a fix which retains all of the present javascript functionality, without the performance overhead. The patch includes more thorough testing for variable access (including via "top.myVar", "parent.myVar" etc), as well as a test that counts the number of calls being made to the javascript engine. Prior to the fix, the "testJavaScriptEngineCallsForVariableAccess" resulted in about 30 separate calls to the JavaScriptEngine. With the fix, the same test results in 5 calls to the JavaScriptEngine. The net result of this patch is that our test times have reduced by an order of magnitude. I hope this will help other having similar performance problems with HtmlUnit. Thanks, Darrell DeBoer ThoughtWorks Australia ---------------------------------------------------------------------- >Comment By: Mike Bowler (mbowler) Date: 2004-09-26 10:51 Message: Logged In: YES user_id=46756 Patch applied - thanks ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=448268&aid=1034897&group_id=47038 ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php Thread at a glance:
Previous Message by Date: click to view message preview[HtmlUnit] CVS Commit: /htmlunit: Fix for performance problem in JavaScriptLog Message: ----------- Fix for performance problem in JavaScript handling. Patch submitted by Darrell DeBoer Modified Files: -------------- htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host: WindowTest.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java) htmlunit/src/xdocs: changes.xml (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/xdocs/changes.xml) htmlunit: project.xml (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/project.xml) htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript: JavaScriptEngineTest.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java) htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/host: Window.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java) Revision Data ------------- Index: WindowTest.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java,v retrieving revision 1.18 retrieving revision 1.19 diff -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java -u -d -r1.18 -r1.19 --- src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java +++ src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java @@ -67,6 +67,7 @@ * @author <a href="mailto:mbowler@xxxxxxxxxxxxxxxxxxxx">Mike Bowler</a> * @author <a href="mailto:chen_jun@xxxxxxxxxxxxxxxxxxxxx">Chen Jun</a> * @author David K. Taylor + * @author Darrell DeBoer */ public class WindowTest extends WebTestCase { /** @@ -910,6 +911,122 @@ } /** + * Variables that are defined inside javascript should be accessible through the + * window object (ie window.myVariable). Test that this works. + * @throws Exception If the test fails. + */ + public void testJavascriptVariableFromTopAndParentFrame() throws Exception { + final WebClient webClient = new WebClient(); + final MockWebConnection webConnection = new MockWebConnection( webClient ); + final List collectedAlerts = new ArrayList(); + + webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts)); + + final String firstContent + = "<html><head><title>First</title></head><body><script>" + + "myVariable = 'first'" + + " </script><iframe name='left' src='http://second' />" + + "</body></html>"; + webConnection.setResponse( + URL_FIRST, firstContent, 200, "OK", "text/html", Collections.EMPTY_LIST ); + + final String secondContent + = "<html><head><title>Second</title></head><body><script>" + + "myVariable = 'second'" + + " </script><iframe name='innermost' src='http://third' />" + + "</body></html>"; + webConnection.setResponse( + URL_SECOND, secondContent,200,"OK","text/html",Collections.EMPTY_LIST ); + + final String thirdContent + = "<html><head><title>Third</title><script>" + + "myVariable = 'third';\n" + + "function doTest() {\n" + + "alert('parent.myVariable = ' + parent.myVariable);\n" + + "alert('top.myVariable = ' + top.myVariable);\n" + + "}\n" + + "</script></head>" + + "<body onload='doTest()'></body></html>"; + + webConnection.setResponse( + URL_THIRD, thirdContent,200,"OK","text/html",Collections.EMPTY_LIST ); + + webClient.setWebConnection( webConnection ); + + final HtmlPage page = (HtmlPage)webClient.getPage( + URL_FIRST, SubmitMethod.POST, Collections.EMPTY_LIST); + assertEquals( "First", page.getTitleText() ); + + final List expectedAlerts = Arrays.asList( new String[]{ + "parent.myVariable = second", + "top.myVariable = first", + } ); + assertEquals( expectedAlerts, collectedAlerts ); + } + /** + * Variables that are defined inside javascript should be accessible through the + * window object (ie window.myVariable). Test that this works. + * @throws Exception If the test fails. + */ + public void testJavascriptVariableFromNamedFrame() throws Exception { + final WebClient webClient = new WebClient(); + final MockWebConnection webConnection = + new MockWebConnection(webClient); + final List collectedAlerts = new ArrayList(); + + webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts)); + + final String firstContent + = "<html><head><title>first</title></head>" + + "<frameset cols='20%,80%'>" + + " <frameset rows='30%,70%'>" + + " <frame src='http://second' name='second'>" + + " <frame src='http://third' name='third'>" + + " </frameset>" + + " <frame src='http://fourth' name='fourth'>" + + "</frameset></html>"; + webConnection.setResponse( URL_FIRST, firstContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + final String secondContent + = "<html><head><title>second</title></head><body><script>" + + "myVariable = 'second';\n" + + "</script><p>second</p></body></html>"; + webConnection.setResponse( URL_SECOND, secondContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + final String thirdContent + = "<html><head><title>third</title></head><body><script>" + + "myVariable = 'third';\n" + + "</script><p>third</p></body></html>"; + webConnection.setResponse( URL_THIRD, thirdContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + final String fourthContent + = "<html><head><title>fourth</title></head><body onload='doTest()'><script>\n" + + "myVariable = 'fourth';\n" + + "function doTest() {\n" + + "alert('parent.second.myVariable = ' + parent.second.myVariable);\n" + + "alert('parent.third.myVariable = ' + parent.third.myVariable);\n" + + "}\n" + + "</script></body></html>"; + webConnection.setResponse( new URL("http://fourth"), fourthContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + webClient.setWebConnection(webConnection); + + final HtmlPage page = (HtmlPage)webClient.getPage( + URL_FIRST, SubmitMethod.POST, Collections.EMPTY_LIST); + assertEquals( "first", page.getTitleText() ); + + final List expectedAlerts = Arrays.asList( new String[]{ + "parent.second.myVariable = second", + "parent.third.myVariable = third", + } ); + assertEquals( expectedAlerts, collectedAlerts ); + } + + /** * Variables that have not been defined should return null when accessed. * @throws Exception If the test fails. */ Index: changes.xml =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/xdocs/changes.xml,v retrieving revision 1.202 retrieving revision 1.203 diff -Lsrc/xdocs/changes.xml -Lsrc/xdocs/changes.xml -u -d -r1.202 -r1.203 --- src/xdocs/changes.xml +++ src/xdocs/changes.xml @@ -176,6 +176,9 @@ Added serialVersionUID to all classes that are Serializable but didn't already have one of these. </action> + <action type="update" dev="mbowler" due-to="" id="1034897"> + Fix for performance problem in JavaScript handling. Patch submitted by Darrell DeBoer + </action> </release> <release version="1.3-pre1" date="November 7, 2003"> Index: project.xml =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/project.xml,v retrieving revision 1.46 retrieving revision 1.47 diff -Lproject.xml -Lproject.xml -u -d -r1.46 -r1.47 --- project.xml +++ project.xml @@ -124,8 +124,10 @@ <name>Christian Sell</name> <email>cse@xxxxxxxxxxx</email> </contributor> + <contributor> + <name>Darrell DeBoer</name> + </contributor> </contributors> - <dependencies> <dependency> <groupId>junit</groupId> Index: JavaScriptEngineTest.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java,v retrieving revision 1.9 retrieving revision 1.10 diff -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java -u -d -r1.9 -r1.10 --- src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java +++ src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java @@ -55,10 +55,13 @@ import com.gargoylesoftware.htmlunit.CollectingAlertHandler; import com.gargoylesoftware.htmlunit.MockWebConnection; +import com.gargoylesoftware.htmlunit.ScriptEngine; import com.gargoylesoftware.htmlunit.SubmitMethod; import com.gargoylesoftware.htmlunit.WebClient; import com.gargoylesoftware.htmlunit.WebTestCase; +import com.gargoylesoftware.htmlunit.html.HtmlAnchor; import com.gargoylesoftware.htmlunit.html.HtmlButtonInput; +import com.gargoylesoftware.htmlunit.html.HtmlElement; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlFrame; import com.gargoylesoftware.htmlunit.html.HtmlPage; @@ -73,6 +76,7 @@ * @version $Revision$ * @author <a href="mailto:mbowler@xxxxxxxxxxxxxxxxxxxx">Mike Bowler</a> * @author Noboru Sinohara + * @author Darrell DeBoer */ public class JavaScriptEngineTest extends WebTestCase { /** @@ -527,10 +531,14 @@ jsContent, 200, "OK", "text/javascript", Collections.EMPTY_LIST ); client.setWebConnection( webConnection ); + final List collectedAlerts = new ArrayList(); + client.setAlertHandler( new CollectingAlertHandler(collectedAlerts) ); + final HtmlPage page = ( HtmlPage )client.getPage( new URL( "http://first/index.html" ), SubmitMethod.POST, Collections.EMPTY_LIST ); assertEquals("foo", page.getTitleText()); + assertEquals( Collections.singletonList("Got to external method"), collectedAlerts ); } @@ -609,8 +617,86 @@ } + /** + * Test that the javascript engine gets called correctly for variable access. + */ + public void testJavaScriptEngineCallsForVariableAccess() throws IOException { + final WebClient client = new WebClient(); + final MockWebConnection webConnection = new MockWebConnection( client ); + + final List collectedAlerts = new ArrayList(); + client.setAlertHandler(new CollectingAlertHandler(collectedAlerts)); + + final String content + = "<html><head><title>foo</title><script>" + + "myDate = 'foo';" + + "function doUnqualifiedVariableAccess() {\n" + + " alert('unqualified: ' + myDate);\n" + + "}\n" + + "function doQualifiedVariableAccess() {\n" + + " alert('qualified: ' + window.myDate);\n" + + "}\n" + + "</script></head><body>" + + "<p>hello world</p>" + + "<a id='unqualified' onclick='doUnqualifiedVariableAccess();'>unqualified</a>" + + "<a id='qualified' onclick='doQualifiedVariableAccess();'>qualified</a>" + + "</body></html>"; + + webConnection.setDefaultResponse( content ); + client.setWebConnection( webConnection ); + final CountingJavaScriptEngine countingJavaScriptEngine = new CountingJavaScriptEngine(client.getScriptEngine()); + client.setScriptEngine(countingJavaScriptEngine); + + final HtmlPage page = ( HtmlPage )client.getPage( + URL_GARGOYLE, + SubmitMethod.POST, Collections.EMPTY_LIST ); + + assertEquals(1, countingJavaScriptEngine.getExecutionCount()); + + ((HtmlAnchor) page.getHtmlElementById("unqualified")).click(); + assertEquals(3, countingJavaScriptEngine.getExecutionCount()); + + ((HtmlAnchor) page.getHtmlElementById("qualified")).click(); + assertEquals(5, countingJavaScriptEngine.getExecutionCount()); + + final List expectedAlerts = Arrays.asList(new String[]{"unqualified: foo", "qualified: foo"}); + assertEquals(expectedAlerts, collectedAlerts); + } + private InputSource createInputSourceForFile( final String fileName ) throws FileNotFoundException { return new InputSource( getFileAsStream(fileName) ); } + + private static final class CountingJavaScriptEngine extends ScriptEngine + { + private ScriptEngine delegate_; + private int scriptExecutionCount = 0; + + protected CountingJavaScriptEngine(ScriptEngine delegate) { + super(delegate.getWebClient()); + delegate_ = delegate; + } + + public void initialize(HtmlPage page) { + delegate_.initialize(page); + } + + public Object execute(HtmlPage htmlPage, String sourceCode, String sourceName, HtmlElement htmlElement) { + scriptExecutionCount++; + return delegate_.execute(htmlPage, sourceCode, sourceName, htmlElement); + } + + public Object callFunction(HtmlPage htmlPage, Object javaScriptFunction, Object thisObject, Object[] args, HtmlElement htmlElementScope) { + return delegate_.callFunction(htmlPage, javaScriptFunction, thisObject, args, htmlElementScope); + } + + public String toString(HtmlPage htmlPage, Object javaScriptObject) { + return delegate_.toString(htmlPage, javaScriptObject); + } + + public int getExecutionCount() { + return scriptExecutionCount; + } + } } Index: Window.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java,v retrieving revision 1.38 retrieving revision 1.39 diff -Lsrc/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java -Lsrc/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java -u -d -r1.38 -r1.39 --- src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java +++ src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java @@ -51,8 +51,6 @@ import com.gargoylesoftware.htmlunit.ConfirmHandler; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.PromptHandler; -import com.gargoylesoftware.htmlunit.ScriptException; -import com.gargoylesoftware.htmlunit.ScriptResult; import com.gargoylesoftware.htmlunit.SubmitMethod; import com.gargoylesoftware.htmlunit.TopLevelWindow; import com.gargoylesoftware.htmlunit.WebWindow; @@ -71,6 +69,7 @@ * @author <a href="mailto:chen_jun@xxxxxxxxxxxxxxxxxxxxx">Chen Jun</a> * @author David K. Taylor * @author <a href="mailto:cse@xxxxxxxxxxx">Christian Sell</a> + * @author Darrell DeBoer */ public final class Window extends SimpleScriptable { @@ -84,13 +83,6 @@ private WindowFramesArray windowFramesArray_; /** - * Kludge to avoid stack overflow problems. This will be set to true if we - * are executing a javascript method to retrieve a value (see getJavaScriptVariable) - * and false otherwise. - */ - private boolean getViaJavaScriptInProgress_ = false; - - /** * Create an instance. The rhino engine requires all host objects * to have a default constructor. */ @@ -542,66 +534,21 @@ setDomNode( document_.getHtmlPage() ); } - Object result; - - final Window window; - if( start == this ) { - result = super.get(name, start); - window = this; - } - else { - result = start.get(name, start); - window = (Window)start; - } + Object result = super.get(name, start); // If we are in a frameset then this might be a frame name if( result == NOT_FOUND ) { final DomNode domNode = ((Window)start).getDomNodeOrNull(); - //System.out.println("Window.get() domNode=["+domNode+"]"); result = getFrameByName( domNode.getPage(), name ); } + // Ask the parent scope, as it may be a variable access like "window.myVar" if( result == NOT_FOUND ) { - result = getJavaScriptVariable(window, name); + result = this.getParentScope().get(name, start); } return result; } - /** - * Use a dynamically created javascript method to retrieve a variable that - * we can't get any other way. - * @param window The window that contains the variable. - * @param name The name of the variable. - * @return The value of the variable or {@link #NOT_FOUND} if it cannot be found. - */ - private Object getJavaScriptVariable( final Window window, final String name ) { - if( window.document_ == null ) { - return NOT_FOUND; - } - - // Set this flag to indicate that we are already calling this method. This - // is to avoid stack overflows. Admittedly ugly but the best I could think - // of. - if( getViaJavaScriptInProgress_ == true ) { - return NOT_FOUND; - } - getViaJavaScriptInProgress_ = true; - try { - final HtmlPage page = window.document_.getHtmlPage(); - final ScriptResult scriptResult = page.executeJavaScriptIfPossible( - "return "+name+";", "variable access for "+name, true, page.getDocumentElement()); - - return scriptResult.getJavaScriptResult(); - } - catch( final ScriptException t ) { - return NOT_FOUND; - } - finally { - getViaJavaScriptInProgress_ = false; - } - } - - private Object getFrameByName( final HtmlPage page, final String name ) { final Iterator iterator = page.getDocumentElement().getAllHtmlChildElements(); ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php Next Message by Date: click to view message preview[HtmlUnit] CVS Commit: SimpleScriptable.java: Fixed PMD warning about unusedLog Message: ----------- Fixed PMD warning about unused variables Modified Files: -------------- htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript: SimpleScriptable.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java) Revision Data ------------- Index: SimpleScriptable.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java,v retrieving revision 1.39 retrieving revision 1.40 diff -Lsrc/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java -Lsrc/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java -u -d -r1.39 -r1.40 --- src/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java +++ src/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java @@ -73,10 +73,15 @@ private Method getter_; private Method setter_; private FunctionObject function_; + + public Method getGetter() { return getter_; } + public Method getSetter() { return setter_; } + public FunctionObject getFunction() { return function_; } + public void setGetter( final Method getter ) { getter_ = getter; } + public void setSetter( final Method setter ) { setter_ = setter; } + public void setFunction( final FunctionObject function ) { function_ = function; } } - - /** * Create an instance. Javascript objects must have a default constructor. */ @@ -178,7 +183,7 @@ final String propertyName = methodName.substring(6); final int state = configuration.getReadablePropertyNameState(clazz, propertyName); if( state == JavaScriptConfiguration.ENABLED ) { - getPropertyInfo(localPropertyMap, propertyName).getter_ = methods[i]; + getPropertyInfo(localPropertyMap, propertyName).setGetter( methods[i] ); } else if( state == JavaScriptConfiguration.NOT_FOUND ) { getLog().trace("Getter for ["+propertyName+"] not found in configuration"); @@ -190,7 +195,7 @@ final String propertyName = methodName.substring(6); final int state = configuration.getWritablePropertyNameState(clazz, propertyName); if( state == JavaScriptConfiguration.ENABLED ) { - getPropertyInfo(localPropertyMap, propertyName).setter_ = methods[i]; + getPropertyInfo(localPropertyMap, propertyName).setSetter( methods[i] ); } else if( state == JavaScriptConfiguration.NOT_FOUND ) { getLog().trace("Setter for ["+propertyName+"] not found in configuration"); @@ -203,7 +208,7 @@ if( state == JavaScriptConfiguration.ENABLED ) { final FunctionObject functionObject = new FunctionObject(functionName, methods[i], this); - getPropertyInfo(localPropertyMap, functionName).function_ = functionObject; + getPropertyInfo(localPropertyMap, functionName).setFunction( functionObject ); } else if( state == JavaScriptConfiguration.NOT_FOUND ) { getLog().trace("Function ["+functionName+"] not found in configuration"); @@ -386,13 +391,13 @@ final Object result; if( propertyNameState == JavaScriptConfiguration.ENABLED ) { - if( info == null || info.getter_ == null ) { + if( info == null || info.getGetter() == null ) { getLog().debug("Getter not implemented for property ["+name+"]"); result = NOT_FOUND; } else { try { - result = info.getter_.invoke( this, new Object[0] ); + result = info.getGetter().invoke( this, new Object[0] ); } catch( final Exception e ) { throw new ScriptException(e); @@ -400,12 +405,12 @@ } } else if( functionNameState == JavaScriptConfiguration.ENABLED ) { - if( info == null || info.function_ == null ) { + if( info == null || info.getFunction() == null ) { getLog().debug("Function not implemented ["+name+"]"); result = NOT_FOUND; } else { - result = info.function_; + result = info.getFunction(); } } else { @@ -436,17 +441,17 @@ final PropertyInfo info = (PropertyInfo)getPropertyMap().get(name); if( propertyNameState == JavaScriptConfiguration.ENABLED ) { - if( info == null || info.setter_ == null ) { + if( info == null || info.getSetter() == null ) { getLog().debug("Setter not implemented for property ["+name+"]"); } else { - final Class parameterClass = info.setter_.getParameterTypes()[0]; + final Class parameterClass = info.getSetter().getParameterTypes()[0]; if( parameterClass == "".getClass() ) { newValue = newValue.toString(); } try { - info.setter_.invoke( - findMatchingScriptable(start, info.setter_), new Object[]{ newValue } ); + info.getSetter().invoke( + findMatchingScriptable(start, info.getSetter()), new Object[]{ newValue } ); } catch( final InvocationTargetException e ) { throw new ScriptException(e.getTargetException()); ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php Previous Message by Thread: click to view message preview[HtmlUnit] [ htmlunit-Patches-1034897 ] Fix for performance problem in JavaScript handlingPatches item #1034897, was opened at 2004-09-26 16:37 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=448268&aid=1034897&group_id=47038 Category: None Group: None Status: Open Resolution: None Priority: 5 Submitted By: Darrell DeBoer (bigdaz) Assigned to: Nobody/Anonymous (nobody) Summary: Fix for performance problem in JavaScript handling Initial Comment: We were having big performance problems using HTMLUnit to test our app, using the latest HTMLUnit from CVS. Some javascript actions would take tens of seconds to execute. Using an older version didn't give us the same trouble. I tracked down the problem, which was introduced when code to handle the "window.myVariable" style of variable access was added. The problem was caused by _all_ javascript variable and function access being routed through the Window.get() method, which then created, registered and executed a new javascript function to access the variable. For example, executing "var myDate = new Date();" would cause HtmlUnit to compile a special "GargoyleWrapper" function to access the built-in Date function. Thus a reasonably simple page could result in thousands of unnecessary round trips to the JavaScriptEngine. Anyway, I've now constructed a fix which retains all of the present javascript functionality, without the performance overhead. The patch includes more thorough testing for variable access (including via "top.myVar", "parent.myVar" etc), as well as a test that counts the number of calls being made to the javascript engine. Prior to the fix, the "testJavaScriptEngineCallsForVariableAccess" resulted in about 30 separate calls to the JavaScriptEngine. With the fix, the same test results in 5 calls to the JavaScriptEngine. The net result of this patch is that our test times have reduced by an order of magnitude. I hope this will help other having similar performance problems with HtmlUnit. Thanks, Darrell DeBoer ThoughtWorks Australia ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=448268&aid=1034897&group_id=47038 ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php Next Message by Thread: click to view message preview[HtmlUnit] CVS Commit: /htmlunit: Fix for performance problem in JavaScriptLog Message: ----------- Fix for performance problem in JavaScript handling. Patch submitted by Darrell DeBoer Modified Files: -------------- htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host: WindowTest.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java) htmlunit/src/xdocs: changes.xml (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/xdocs/changes.xml) htmlunit: project.xml (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/project.xml) htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript: JavaScriptEngineTest.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java) htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/host: Window.java (http://cvs.sourceforge.net/viewcvs.py/htmlunit/htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java) Revision Data ------------- Index: WindowTest.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java,v retrieving revision 1.18 retrieving revision 1.19 diff -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java -u -d -r1.18 -r1.19 --- src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java +++ src/test/java/com/gargoylesoftware/htmlunit/javascript/host/WindowTest.java @@ -67,6 +67,7 @@ * @author <a href="mailto:mbowler@xxxxxxxxxxxxxxxxxxxx">Mike Bowler</a> * @author <a href="mailto:chen_jun@xxxxxxxxxxxxxxxxxxxxx">Chen Jun</a> * @author David K. Taylor + * @author Darrell DeBoer */ public class WindowTest extends WebTestCase { /** @@ -910,6 +911,122 @@ } /** + * Variables that are defined inside javascript should be accessible through the + * window object (ie window.myVariable). Test that this works. + * @throws Exception If the test fails. + */ + public void testJavascriptVariableFromTopAndParentFrame() throws Exception { + final WebClient webClient = new WebClient(); + final MockWebConnection webConnection = new MockWebConnection( webClient ); + final List collectedAlerts = new ArrayList(); + + webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts)); + + final String firstContent + = "<html><head><title>First</title></head><body><script>" + + "myVariable = 'first'" + + " </script><iframe name='left' src='http://second' />" + + "</body></html>"; + webConnection.setResponse( + URL_FIRST, firstContent, 200, "OK", "text/html", Collections.EMPTY_LIST ); + + final String secondContent + = "<html><head><title>Second</title></head><body><script>" + + "myVariable = 'second'" + + " </script><iframe name='innermost' src='http://third' />" + + "</body></html>"; + webConnection.setResponse( + URL_SECOND, secondContent,200,"OK","text/html",Collections.EMPTY_LIST ); + + final String thirdContent + = "<html><head><title>Third</title><script>" + + "myVariable = 'third';\n" + + "function doTest() {\n" + + "alert('parent.myVariable = ' + parent.myVariable);\n" + + "alert('top.myVariable = ' + top.myVariable);\n" + + "}\n" + + "</script></head>" + + "<body onload='doTest()'></body></html>"; + + webConnection.setResponse( + URL_THIRD, thirdContent,200,"OK","text/html",Collections.EMPTY_LIST ); + + webClient.setWebConnection( webConnection ); + + final HtmlPage page = (HtmlPage)webClient.getPage( + URL_FIRST, SubmitMethod.POST, Collections.EMPTY_LIST); + assertEquals( "First", page.getTitleText() ); + + final List expectedAlerts = Arrays.asList( new String[]{ + "parent.myVariable = second", + "top.myVariable = first", + } ); + assertEquals( expectedAlerts, collectedAlerts ); + } + /** + * Variables that are defined inside javascript should be accessible through the + * window object (ie window.myVariable). Test that this works. + * @throws Exception If the test fails. + */ + public void testJavascriptVariableFromNamedFrame() throws Exception { + final WebClient webClient = new WebClient(); + final MockWebConnection webConnection = + new MockWebConnection(webClient); + final List collectedAlerts = new ArrayList(); + + webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts)); + + final String firstContent + = "<html><head><title>first</title></head>" + + "<frameset cols='20%,80%'>" + + " <frameset rows='30%,70%'>" + + " <frame src='http://second' name='second'>" + + " <frame src='http://third' name='third'>" + + " </frameset>" + + " <frame src='http://fourth' name='fourth'>" + + "</frameset></html>"; + webConnection.setResponse( URL_FIRST, firstContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + final String secondContent + = "<html><head><title>second</title></head><body><script>" + + "myVariable = 'second';\n" + + "</script><p>second</p></body></html>"; + webConnection.setResponse( URL_SECOND, secondContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + final String thirdContent + = "<html><head><title>third</title></head><body><script>" + + "myVariable = 'third';\n" + + "</script><p>third</p></body></html>"; + webConnection.setResponse( URL_THIRD, thirdContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + final String fourthContent + = "<html><head><title>fourth</title></head><body onload='doTest()'><script>\n" + + "myVariable = 'fourth';\n" + + "function doTest() {\n" + + "alert('parent.second.myVariable = ' + parent.second.myVariable);\n" + + "alert('parent.third.myVariable = ' + parent.third.myVariable);\n" + + "}\n" + + "</script></body></html>"; + webConnection.setResponse( new URL("http://fourth"), fourthContent, + 200, "OK", "text/html", Collections.EMPTY_LIST); + + webClient.setWebConnection(webConnection); + + final HtmlPage page = (HtmlPage)webClient.getPage( + URL_FIRST, SubmitMethod.POST, Collections.EMPTY_LIST); + assertEquals( "first", page.getTitleText() ); + + final List expectedAlerts = Arrays.asList( new String[]{ + "parent.second.myVariable = second", + "parent.third.myVariable = third", + } ); + assertEquals( expectedAlerts, collectedAlerts ); + } + + /** * Variables that have not been defined should return null when accessed. * @throws Exception If the test fails. */ Index: changes.xml =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/xdocs/changes.xml,v retrieving revision 1.202 retrieving revision 1.203 diff -Lsrc/xdocs/changes.xml -Lsrc/xdocs/changes.xml -u -d -r1.202 -r1.203 --- src/xdocs/changes.xml +++ src/xdocs/changes.xml @@ -176,6 +176,9 @@ Added serialVersionUID to all classes that are Serializable but didn't already have one of these. </action> + <action type="update" dev="mbowler" due-to="" id="1034897"> + Fix for performance problem in JavaScript handling. Patch submitted by Darrell DeBoer + </action> </release> <release version="1.3-pre1" date="November 7, 2003"> Index: project.xml =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/project.xml,v retrieving revision 1.46 retrieving revision 1.47 diff -Lproject.xml -Lproject.xml -u -d -r1.46 -r1.47 --- project.xml +++ project.xml @@ -124,8 +124,10 @@ <name>Christian Sell</name> <email>cse@xxxxxxxxxxx</email> </contributor> + <contributor> + <name>Darrell DeBoer</name> + </contributor> </contributors> - <dependencies> <dependency> <groupId>junit</groupId> Index: JavaScriptEngineTest.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java,v retrieving revision 1.9 retrieving revision 1.10 diff -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java -Lsrc/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java -u -d -r1.9 -r1.10 --- src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java +++ src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngineTest.java @@ -55,10 +55,13 @@ import com.gargoylesoftware.htmlunit.CollectingAlertHandler; import com.gargoylesoftware.htmlunit.MockWebConnection; +import com.gargoylesoftware.htmlunit.ScriptEngine; import com.gargoylesoftware.htmlunit.SubmitMethod; import com.gargoylesoftware.htmlunit.WebClient; import com.gargoylesoftware.htmlunit.WebTestCase; +import com.gargoylesoftware.htmlunit.html.HtmlAnchor; import com.gargoylesoftware.htmlunit.html.HtmlButtonInput; +import com.gargoylesoftware.htmlunit.html.HtmlElement; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlFrame; import com.gargoylesoftware.htmlunit.html.HtmlPage; @@ -73,6 +76,7 @@ * @version $Revision$ * @author <a href="mailto:mbowler@xxxxxxxxxxxxxxxxxxxx">Mike Bowler</a> * @author Noboru Sinohara + * @author Darrell DeBoer */ public class JavaScriptEngineTest extends WebTestCase { /** @@ -527,10 +531,14 @@ jsContent, 200, "OK", "text/javascript", Collections.EMPTY_LIST ); client.setWebConnection( webConnection ); + final List collectedAlerts = new ArrayList(); + client.setAlertHandler( new CollectingAlertHandler(collectedAlerts) ); + final HtmlPage page = ( HtmlPage )client.getPage( new URL( "http://first/index.html" ), SubmitMethod.POST, Collections.EMPTY_LIST ); assertEquals("foo", page.getTitleText()); + assertEquals( Collections.singletonList("Got to external method"), collectedAlerts ); } @@ -609,8 +617,86 @@ } + /** + * Test that the javascript engine gets called correctly for variable access. + */ + public void testJavaScriptEngineCallsForVariableAccess() throws IOException { + final WebClient client = new WebClient(); + final MockWebConnection webConnection = new MockWebConnection( client ); + + final List collectedAlerts = new ArrayList(); + client.setAlertHandler(new CollectingAlertHandler(collectedAlerts)); + + final String content + = "<html><head><title>foo</title><script>" + + "myDate = 'foo';" + + "function doUnqualifiedVariableAccess() {\n" + + " alert('unqualified: ' + myDate);\n" + + "}\n" + + "function doQualifiedVariableAccess() {\n" + + " alert('qualified: ' + window.myDate);\n" + + "}\n" + + "</script></head><body>" + + "<p>hello world</p>" + + "<a id='unqualified' onclick='doUnqualifiedVariableAccess();'>unqualified</a>" + + "<a id='qualified' onclick='doQualifiedVariableAccess();'>qualified</a>" + + "</body></html>"; + + webConnection.setDefaultResponse( content ); + client.setWebConnection( webConnection ); + final CountingJavaScriptEngine countingJavaScriptEngine = new CountingJavaScriptEngine(client.getScriptEngine()); + client.setScriptEngine(countingJavaScriptEngine); + + final HtmlPage page = ( HtmlPage )client.getPage( + URL_GARGOYLE, + SubmitMethod.POST, Collections.EMPTY_LIST ); + + assertEquals(1, countingJavaScriptEngine.getExecutionCount()); + + ((HtmlAnchor) page.getHtmlElementById("unqualified")).click(); + assertEquals(3, countingJavaScriptEngine.getExecutionCount()); + + ((HtmlAnchor) page.getHtmlElementById("qualified")).click(); + assertEquals(5, countingJavaScriptEngine.getExecutionCount()); + + final List expectedAlerts = Arrays.asList(new String[]{"unqualified: foo", "qualified: foo"}); + assertEquals(expectedAlerts, collectedAlerts); + } + private InputSource createInputSourceForFile( final String fileName ) throws FileNotFoundException { return new InputSource( getFileAsStream(fileName) ); } + + private static final class CountingJavaScriptEngine extends ScriptEngine + { + private ScriptEngine delegate_; + private int scriptExecutionCount = 0; + + protected CountingJavaScriptEngine(ScriptEngine delegate) { + super(delegate.getWebClient()); + delegate_ = delegate; + } + + public void initialize(HtmlPage page) { + delegate_.initialize(page); + } + + public Object execute(HtmlPage htmlPage, String sourceCode, String sourceName, HtmlElement htmlElement) { + scriptExecutionCount++; + return delegate_.execute(htmlPage, sourceCode, sourceName, htmlElement); + } + + public Object callFunction(HtmlPage htmlPage, Object javaScriptFunction, Object thisObject, Object[] args, HtmlElement htmlElementScope) { + return delegate_.callFunction(htmlPage, javaScriptFunction, thisObject, args, htmlElementScope); + } + + public String toString(HtmlPage htmlPage, Object javaScriptObject) { + return delegate_.toString(htmlPage, javaScriptObject); + } + + public int getExecutionCount() { + return scriptExecutionCount; + } + } } Index: Window.java =================================================================== RCS file: /cvsroot/htmlunit/htmlunit/src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java,v retrieving revision 1.38 retrieving revision 1.39 diff -Lsrc/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java -Lsrc/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java -u -d -r1.38 -r1.39 --- src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java +++ src/java/com/gargoylesoftware/htmlunit/javascript/host/Window.java @@ -51,8 +51,6 @@ import com.gargoylesoftware.htmlunit.ConfirmHandler; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.PromptHandler; -import com.gargoylesoftware.htmlunit.ScriptException; -import com.gargoylesoftware.htmlunit.ScriptResult; import com.gargoylesoftware.htmlunit.SubmitMethod; import com.gargoylesoftware.htmlunit.TopLevelWindow; import com.gargoylesoftware.htmlunit.WebWindow; @@ -71,6 +69,7 @@ * @author <a href="mailto:chen_jun@xxxxxxxxxxxxxxxxxxxxx">Chen Jun</a> * @author David K. Taylor * @author <a href="mailto:cse@xxxxxxxxxxx">Christian Sell</a> + * @author Darrell DeBoer */ public final class Window extends SimpleScriptable { @@ -84,13 +83,6 @@ private WindowFramesArray windowFramesArray_; /** - * Kludge to avoid stack overflow problems. This will be set to true if we - * are executing a javascript method to retrieve a value (see getJavaScriptVariable) - * and false otherwise. - */ - private boolean getViaJavaScriptInProgress_ = false; - - /** * Create an instance. The rhino engine requires all host objects * to have a default constructor. */ @@ -542,66 +534,21 @@ setDomNode( document_.getHtmlPage() ); } - Object result; - - final Window window; - if( start == this ) { - result = super.get(name, start); - window = this; - } - else { - result = start.get(name, start); - window = (Window)start; - } + Object result = super.get(name, start); // If we are in a frameset then this might be a frame name if( result == NOT_FOUND ) { final DomNode domNode = ((Window)start).getDomNodeOrNull(); - //System.out.println("Window.get() domNode=["+domNode+"]"); result = getFrameByName( domNode.getPage(), name ); } + // Ask the parent scope, as it may be a variable access like "window.myVar" if( result == NOT_FOUND ) { - result = getJavaScriptVariable(window, name); + result = this.getParentScope().get(name, start); } return result; } - /** - * Use a dynamically created javascript method to retrieve a variable that - * we can't get any other way. - * @param window The window that contains the variable. - * @param name The name of the variable. - * @return The value of the variable or {@link #NOT_FOUND} if it cannot be found. - */ - private Object getJavaScriptVariable( final Window window, final String name ) { - if( window.document_ == null ) { - return NOT_FOUND; - } - - // Set this flag to indicate that we are already calling this method. This - // is to avoid stack overflows. Admittedly ugly but the best I could think - // of. - if( getViaJavaScriptInProgress_ == true ) { - return NOT_FOUND; - } - getViaJavaScriptInProgress_ = true; - try { - final HtmlPage page = window.document_.getHtmlPage(); - final ScriptResult scriptResult = page.executeJavaScriptIfPossible( - "return "+name+";", "variable access for "+name, true, page.getDocumentElement()); - - return scriptResult.getJavaScriptResult(); - } - catch( final ScriptException t ) { - return NOT_FOUND; - } - finally { - getViaJavaScriptInProgress_ = false; - } - } - - private Object getFrameByName( final HtmlPage page, final String name ) { final Iterator iterator = page.getDocumentElement().getAllHtmlChildElements(); ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php Loading Comments...
|
|
