What performance improvement do you achieve? I’d be surprised if you reach 1%, and even so that wouldn’t be worth the duplication of the test, along with the decreased maintainability.
Surely there are parts of the code architecture that can be improved and lead to better savings. This one looks a bit to me like a desperate move after everything else has been done. I don’t quite think we are there yet... Vincent On 21/01/13 23:34, Glenn Adams wrote: > One more point, which is "probably inlined" is not a very good guarantee, > since that is strictly an implementation option. Better to error on the > side of assuming it won't inline, and at least optimize the hot spots that > would be affect if inlining doesn't occur. > > On Mon, Jan 21, 2013 at 12:12 PM, Glenn Adams <gl...@skynav.com> wrote: > >> I agree that when JIT inlines this it will not have much affect. However, >> when profiling, this was identified as a hot spot. >> >> >> On Mon, Jan 21, 2013 at 11:52 AM, Vincent Hennebert >> <vhenneb...@gmail.com>wrote: >> >>> Hi Glenn, >>> >>> does that significantly improve performance? I would be really surprised >>> if it would, given that the method call is probably inlined by the JIT >>> compiler anyway. Do you have figures to share? >>> >>> Thanks, >>> Vincent >>> >>> >>> On 18/01/13 22:13, gadams wrote: >>>> Author: gadams >>>> Date: Fri Jan 18 21:13:42 2013 >>>> New Revision: 1435344 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1435344&view=rev >>>> Log: >>>> FOP-2194: optimize lazy font load invocation for hot methods >>>> >>>> Modified: >>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/fonts/LazyFont.java >>>> xmlgraphics/fop/trunk/status.xml >>>> >>>> Modified: >>> xmlgraphics/fop/trunk/src/java/org/apache/fop/fonts/LazyFont.java >>>> URL: >>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fonts/LazyFont.java?rev=1435344&r1=1435343&r2=1435344&view=diff >>>> >>> ============================================================================== >>>> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/fonts/LazyFont.java >>> (original) >>>> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fonts/LazyFont.java >>> Fri Jan 18 21:13:42 2013 >>>> @@ -153,7 +153,9 @@ public class LazyFont extends Typeface i >>>> * {@inheritDoc} >>>> */ >>>> public char mapChar(char c) { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> return realFont.mapChar(c); >>>> } >>>> >>>> @@ -169,7 +171,9 @@ public class LazyFont extends Typeface i >>>> * {@inheritDoc} >>>> */ >>>> public boolean hasChar(char c) { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> return realFont.hasChar(c); >>>> } >>>> >>>> @@ -250,7 +254,9 @@ public class LazyFont extends Typeface i >>>> * {@inheritDoc} >>>> */ >>>> public int getWidth(int i, int size) { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> return realFont.getWidth(i, size); >>>> } >>>> >>>> @@ -384,7 +390,9 @@ public class LazyFont extends Typeface i >>>> */ >>>> public CharSequence reorderCombiningMarks >>>> ( CharSequence cs, int[][] gpa, String script, String language >>> ) { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> if ( realFontDescriptor instanceof Substitutable ) { >>>> return ((Substitutable)realFontDescriptor) >>>> .reorderCombiningMarks(cs, gpa, script, language); >>>> @@ -397,7 +405,9 @@ public class LazyFont extends Typeface i >>>> * {@inheritDoc} >>>> */ >>>> public boolean performsPositioning() { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> if ( realFontDescriptor instanceof Positionable ) { >>>> return >>> ((Positionable)realFontDescriptor).performsPositioning(); >>>> } else { >>>> @@ -410,7 +420,9 @@ public class LazyFont extends Typeface i >>>> */ >>>> public int[][] >>>> performPositioning ( CharSequence cs, String script, String >>> language, int fontSize ) { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> if ( realFontDescriptor instanceof Positionable ) { >>>> return ((Positionable)realFontDescriptor) >>>> .performPositioning(cs, script, language, fontSize); >>>> @@ -424,7 +436,9 @@ public class LazyFont extends Typeface i >>>> */ >>>> public int[][] >>>> performPositioning ( CharSequence cs, String script, String >>> language ) { >>>> - load(true); >>>> + if ( !isMetricsLoaded ) { >>>> + load(true); >>>> + } >>>> if ( realFontDescriptor instanceof Positionable ) { >>>> return ((Positionable)realFontDescriptor) >>>> .performPositioning(cs, script, language); >>>> >>>> Modified: xmlgraphics/fop/trunk/status.xml >>>> URL: >>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1435344&r1=1435343&r2=1435344&view=diff >>>> >>> ============================================================================== >>>> --- xmlgraphics/fop/trunk/status.xml (original) >>>> +++ xmlgraphics/fop/trunk/status.xml Fri Jan 18 21:13:42 2013 >>>> @@ -59,6 +59,9 @@ >>>> documents. Example: the fix of marks layering will be such a >>> case when it's done. >>>> --> >>>> <release version="FOP Trunk" date="TBD"> >>>> + <action context="Fonts" dev="GA" type="fix" fixes-bug="FOP-2194"> >>>> + Optimize lazy font load invocation for hot methods. >>>> + </action> >>>> <action context="Code" dev="GA" type="fix" fixes-bug="FOP-2192"> >>>> Fix checkstyle and findbugs warnings. >>>> </action> >>>> >>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: fop-commits-unsubscr...@xmlgraphics.apache.org >>>> For additional commands, e-mail: >>> fop-commits-h...@xmlgraphics.apache.org >>>> >>> >> >> >
