User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:126.96.36.199) Gecko/20080201 Firefox/188.8.131.52 Build Identifier: 1.6R5 There seems to be a problem between 1.5 and 1.6 release when using importPackage(). Reproducible: Always Steps to Reproduce: For below snippet: importPackage(java.util); var searchmon = application.data["month"].value; var searchday = application.data["day"].value; var searchyear = application.data["year"].value; var searchwkday = application.data["wkDay"]; var myDate = Calendar.getInstance(); myDate.set(Calendar.MONTH, searchmon); myDate.set(Calendar.DATE, searchday); myDate.set(Calendar.YEAR, searchyear); searchwkday.value = myDate.get(Calendar.DAY_OF_WEEK); Actual Results: An approximate performance drop is at magnitude of 10. Once you replace importPackage() with importClass(), there is no performance issue. Expected Results: The performance is the same or better with newer release (1.6x) After investigation it turned out the problem is either in caching inside the NativeJavaPackage.getPkgProperty(String name, Scriptable start, boolean createPkg), or in the general approach in the TopLevelScope. It looks like EVERY variable goes through the package evaluation process whenever importPackage has been used. The getPkgProperty() takes care of the previously FOUND classes but does not cache the information about classes that HAVE NOT BEEN FOUND, thus searching over and over for the classes that does not exist and failed previously. As a workaround I used my own class loaded that caches the information about not found classes and fails fast. However, either the clarification on the actual intent (read - 'yes, we want to dynamically check for new classes') or the fix is needed here.
By TopLevelScope I meant ImporterTopLevel.
Looking at the latest CVS source, I'm not seeing this behavior. I put a breakpoint on NativeJavaPackage.getPkgProperty, then access a globally defined variable, both before and after executing importPackage. I don't get any calls to NativeJavaPackage.getPkgProperty for variables that are defined at the top level. In your example code, I was assuming that "application" and "Calendar" were defined by your application. Are you using shared scopes or any other configuration that might come into play here? And as far as intent, I was not trying to detect dynamically loaded classes by repeatedly attempting to load them. I'm fine with Rhino only attempting to load a class once and upon failure thereafter assuming it doesn't exist.
Created attachment 312652 [details] [diff] [review] Proposed patch Thanks--that test case illustrated what was going on. When a script is first started, Rhino must look up variables defined in the script and merge them with previous definitions. To perform this merge it has to look them up in the scope, and with importerTopLevel having package imports it causes a call to getPkgProperty for each variable. The first call to runTestScript didn't trigger this behavior because the importPackage calls had not yet been made. I think the best option here is, as you suggest, to maintain a cache of misses so we don't retry the class lookup. I've attached a proposed patch. Can you let me know if this change addresses the performance issue? Thanks for your help.
Assignee: nobody → norrisboyd
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
I tried the patch and it works fine now. Thank you!
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Which is the latest release that has this fix. we are facing the same issue.
I think the fix to this bug caused a regression, but as it's so old, I created a new report. Please see bug 694286.
You need to log in before you can comment on or make changes to this bug.