Project: Birt
Bug 102498 Script execution performance problems
Reported: 2005-07-01 11:46:42 | Status: resolved fixed

While testing the suitability of the BIRT report engine on the IBM iSeries, unexpectedly poor performance was encountered. This was especially apparent with the simple chart example report that is attached to this bug report. After doing a bit of profiling using TPTP, the cause became apparent. The method call stack causing the performance problem is as follows: org.eclipse.birt.data.engine.executor.CachedResultSet#init org.eclipse.birt.data.engine.executor.CachedResultSet#copyAndFilterDataRows(Object) org.eclipse.birt.data.engine.executor.CachedResultSet#processFetchEvent(IResultObject) org.eclipse.birt.data.engine.impl.FilterByRow#process(IResultObject) org.eclipse.birt.data.engine.script.ScriptEvalUtil#evalExpr(IBaseExpression, Context, Scriptable, String, int) org.eclipse.birt.data.engine.script.ScriptEvalUtil#evaluateJSAsExpr(Context, Scriptable, String, String, int) org.eclipse.birt.data.engine.script.ScriptEvalUtil#evaluateJSScript(Context, Scriptable, String, String, int) org.mozilla.javascript.Context#evaluateString(Scriptable, String, String, int, Object) In this case, Rhino will compile the specified Javascript string into an optimized Java class that implements that script. It then runs the compiled script class and once it is done with the execution, the class instance and definition is thrown away. This happens over and over, costing time in the parsing, class generation and class load. In the simple (attached) case, there were only 5 unique scripts that were being run via this method and yet over 500 Javascript implementation classes were generated due to the number of times the script evaluation code was executed for those scripts. It is unclear why this particular path evaluates the string on each call rather than caching the compiled script results from Rhino and executing the compiled script instead. The class org.eclipse.birt.data.engine.impl.CompiledExpression and its subclasses shows that the BIRT team understands the need for compilation and reuse in other parts of the system. There are obviously many approaches that could be used to fix this problem. The most architecturally sound approach would be to have the FilterByRow (and more abstractly, all implementations of org.eclipse.birt.data.engine.odi.IResultObjectEvent to compile and cache the necessary script object rather than always evaluating the string. It is unclear to us with our limited knowledge of the code just how widespread this problem is within the BIRT codebase and therefore how much work this would be to implement. A simpler, less elegant, solution would be to build a Least Recently Used cache into ScriptEvalUtil to cache the recent script evaluations based on the text of the script. To test the performance improvement gained by caching the compiled script, a naive change was made to the evaluateJSScript method to compile and cache the string to be evaluated: private static Object evaluateJSScript(Context cx, Scriptable scope, String scriptText, String source, int lineNo) throws DataException { Object result = null; try { Script script = (Script) scriptCache.get(scriptText); if (script == null) { script = cx.compileString(scriptText, source, lineNo, null); scriptCache.put(scriptText, script); } result = script.exec(cx, scope); // result = cx.evaluateString(scope, scriptText, source, lineNo, null); // It seems Rhino 1.6 has changed its way to process incorrect expression. // When there is an error, exception will not be thrown, but rather an Undefined // instance will be returned. Here its return value is changed to null. if ( result instanceof Undefined ) { //throw new Exception( scriptText + " is not valid expression." ); return null; } } catch ( RhinoException e) { RethrowJSEvalException( e, scriptText, source, lineNo ); } return convertNativeObjToJavaObj(result); } This change showed significant improvement on the attached report rendering (times in minutes/seconds): Windows (1.3Ghz) Windows iSeries iSeries without change with change without change with change First 0:14 0:08 1:28 0:42 Load Reload 0:04 0:02 0:35 0:05 As you can see, this had a huge impact on both the first touch render of the chart as well as the reload of the report. This naive implementation does not take into account the fact that there may be an ever-growing script cache, but does prove that caching the results using LRU or similar will have a significant positive impact. Please do whatever you possibly can to get a performance fix in place for 1.0.1. If it is not possible to do the most architecturally sound approach in that timeframe, an LRU cache would be a good second choice. If there is anything more that we can do to see these changes implemented, please let us know.

Files changed in fix:
data/org.eclipse.birt.data/src/org/eclipse/birt/data/engine/script/ScriptEvalUtil.java