review for 7071307: MethodHandle bimorphic inlining should consider the frequency

[Tom Rodriguez]

I'll fix that as you suggest.

diff -r a19c671188cb src/share/vm/ci/ciCallProfile.hpp                                                                                                
--- a/src/share/vm/ci/ciCallProfile.hpp                                                                                                              
+++ b/src/share/vm/ci/ciCallProfile.hpp                                                                                                              
@@ -79,6 +79,17 @@
     assert(i < _limit, "out of Call Profile MorphismLimit");                                                                                        
     return _receiver[i];                                                                                                                            
   }                                                                                                                                                  
+                                                                                                                                                    
+  // Rescale the current profile based on the incoming scale                                                                                        
+  ciCallProfile rescale(double scale) {                                                                                                              
+    assert(scale >= 0 && scale <= 1.0, "out of range");                                                                                              
+    ciCallProfile call = *this;                                                                                                                      
+    call._count = (int)(call._count * scale);                                                                                                        
+    for (int i = 0; i < _morphism; i++) {                                                                                                            
+      call._receiver_count[i] = (int)(call._receiver_count[i] * scale);                                                                              
+    }                                                                                                                                                
+    return call;                                                                                                                                    
+  }                                                                                                                                                  
 };                                                                                                                                                  
                                                                                                                                                     
 #endif // SHARE_VM_CI_CICALLPROFILE_HPP

I haven't pushed this yet because I was seeing some cases where the if's were ordered how I expect and I'm still trying to figure out if this is me or something odd in jruby.  I should get to the bottom of this today.

tom

On Aug 8, 2011, at 2:29 PM, Charles Oliver Nutter wrote:

> On Thu, Jul 28, 2011 at 7:47 PM, Tom Rodriguez <tom.rodriguez at oracle.com> wrote:
>> http://cr.openjdk.java.net/~never/7071307
>> 46 lines changed: 27 ins; 6 del; 13 mod; 3568 unchg
>> 
>> 7071307: MethodHandle bimorphic inlining should consider the frequency
>> Reviewed-by:
>> 
>> The fix for 7050554 added a bimorphic inline path but didn't take into
>> account the frequency of the guarding test.  This ends up treating
>> both sides of the if as equally frequent which can lead to over
>> inlining and overflowing the method inlining limits.  The fix is to
>> grab the frequency from the If and apply that to the branches.  This
>> addresses a major source of overinlining that can result in bad
>> performance with JSR 292.  We may do a later extension to this to
>> actually do per call chain profiling of selectAlternative but that's a
>> more complicated fix.
>> 
>> I also fixed a problem with the ideal graph printer where debug_orig
>> printing would go into an infinite loop.
>> 
>> Tested with jruby and vm.mlvm tests.
> 
> Building on Ubuntu (an admittedly old install) yields some warnings
> that are turned into errors:
> 
> g++ -DLINUX -D_GNU_SOURCE -DIA32 -DPRODUCT -I.
> -I/home/headius/hsx-hotspot/src/share/vm/prims
> -I/home/headius/hsx-hotspot/src/share/vm
> -I/home/headius/hsx-hotspot/src/cpu/x86/vm
> -I/home/headius/hsx-hotspot/src/os_cpu/linux_x86/vm
> -I/home/headius/hsx-hotspot/src/os/linux/vm
> -I/home/headius/hsx-hotspot/src/os/posix/vm -I../generated
> -DHOTSPOT_RELEASE_VERSION="\"22.0-b01-internal\""
> -DHOTSPOT_BUILD_TARGET="\"product\""
> -DHOTSPOT_BUILD_USER="\"headius\"" -DHOTSPOT_LIB_ARCH=\"i386\"
> -DJRE_RELEASE_VERSION="\"1.7.0\"" -DHOTSPOT_VM_DISTRO="\"OpenJDK\""
> -DTARGET_OS_FAMILY_linux -DTARGET_ARCH_x86 -DTARGET_ARCH_MODEL_x86_32
> -DTARGET_OS_ARCH_linux_x86 -DTARGET_OS_ARCH_MODEL_linux_x86_32
> -DTARGET_COMPILER_gcc -DCOMPILER2 -DCOMPILER1 -fPIC -fno-rtti
> -fno-exceptions -D_REENTRANT -fcheck-new -m32 -march=i586 -pipe -O3
> -fno-strict-aliasing -DVM_LITTLE_ENDIAN -Werror -Wpointer-arith
> -Wconversion -Wsign-compare    -c -MMD -MP -MF
> ../generated/dependencies/precompiled.hpp.gch.d -x c++-header
> /home/headius/hsx-hotspot/src/share/vm/precompiled.hpp -o
> precompiled.hpp.gch
> cc1plus: warnings being treated as errors
> /home/headius/hsx-hotspot/src/share/vm/ci/ciCallProfile.hpp: In member
> function 'ciCallProfile ciCallProfile::rescale(double)':
> /home/headius/hsx-hotspot/src/share/vm/ci/ciCallProfile.hpp:87:
> warning: converting to 'int' from 'double'
> /home/headius/hsx-hotspot/src/share/vm/ci/ciCallProfile.hpp:89:
> warning: converting to 'int' from 'double'
> 
> The lines in question are doing (int) *= (double), which gcc complains
> about. Ubuntu probably has warnings set up to be errors, so it fails
> the build.
> 
> I modified them in my local copy to do the long form with an explicit
> cast back to int, but you can fix in whatever way is best.
> 
> - Charlie