----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9079/ -----------------------------------------------------------
(Updated Jan. 25, 2013, 3:46 a.m.) Review request for oozie. Changes ------- To understand the utility of using Graphics.dispose(), I studied some of this code in detail. Here's some data to back the changes in the patch. Although, the javadocs indicate that we do not need to invoke g.dispose() explicitly, looking carefully into the Component class's paint() implementation, note how it only disposes the new graphics object it creates (co) using the Graphics context and not the original one (g). http://javasourcecode.org/html/open-source/jdk/jdk-6u23/javax/swing/JComponent.java.html (lines 967-1053) bq public void paint(Graphics g) { boolean shouldClearPaintFlags = false; if ((getWidth() <= 0) || (getHeight() <= 0)) { return; } Graphics componentGraphics = getComponentGraphics(g); Graphics co = componentGraphics.create(); .... finally { co.dispose(); bq. Therefore, in our GraphGenerator code, as we are creating a new Graphics object, we alone are responsible for disposing it. The superimplementation of paint() in the JUNG library's classes e.g.VisualizationImageServer are not seen to override paint() in a way to do dispose() either. The heap memory profiling clearly shows the changes with and w/o the dispose line. Without it, heap occupied keeps monotonically rising with repeated job requests, dangerously in the ballpark of GBs, till GC hits. With patch, only spikes with each request but returns to low immediately and stays in MBs. Excluded the unit test as it was flaky between different run environments. But applying formatter introduced few changes to that file. Other review comments addressed. Description ------- https://issues.apache.org/jira/browse/OOZIE-1186 This addresses bug OOZIE-1186. https://issues.apache.org/jira/browse/OOZIE-1186 Diffs (updated) ----- trunk/core/src/main/java/org/apache/oozie/util/GraphGenerator.java 1437616 trunk/core/src/test/java/org/apache/oozie/util/TestGraphGenerator.java 1437616 Diff: https://reviews.apache.org/r/9079/diff/ Testing ------- unit test done + end-to-end using Yourkit memory profiler Thanks, Mona Chitnis
