JDK-8202332 : Empty methods should be considered trivial by SimpleThresholdPolicy
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11
  • Priority: P4
  • Status: Closed
  • Resolution: Not an Issue
  • Submitted: 2018-04-26
  • Updated: 2019-01-15
  • Resolved: 2019-01-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdResolved
Related Reports
Relates :  
Description
Adding method->is_empty_method() to SimpleThresholdPolicy::is_trivial(Method* method) would allow tiered compilation to shortcut an unnecessary pass through tier 3:

diff -r 770679787db5 src/hotspot/share/runtime/simpleThresholdPolicy.inline.hpp
--- a/src/hotspot/share/runtime/simpleThresholdPolicy.inline.hpp	Thu Apr 26 17:14:04 2018 +0200
+++ b/src/hotspot/share/runtime/simpleThresholdPolicy.inline.hpp	Thu Apr 26 18:13:46 2018 +0200
@@ -73,7 +73,8 @@
 // Determine if a given method is such a case.
 bool SimpleThresholdPolicy::is_trivial(Method* method) {
   if (method->is_accessor() ||
-      method->is_constant_getter()) {
+      method->is_constant_getter() ||
+      method->is_empty_method()) {
     return true;
   }
 #if INCLUDE_JVMCI

The current Method::is_empty_method implicitly avoids treating Object.<init> as a trivial method, so it's possible that it'd be worthwhile to add an alternative method that would include Object.<init> by using java_code_at(0) == Bytecodes::_return rather than *code_base() == Bytecodes::_return
Comments
The trivial heuristic has been linked to local regressions and is being phased out, so it doesn't seem appropriate to widen the heuristic at this point.
09-01-2019

Thanks for the pointer, [~dlong]! It seems then that we have regressed some time between now and JDK-8145579 since Object::<init> never gets done by C2 currently: ObjectInit.nonTiered avgt 10 22.745 �� 0.056 ns/op ObjectInit.tiered avgt 10 23.988 �� 0.028 ns/op (The patch ideas floated here does not change the score for ObjectInit.tiered) I'll file a bug to investigate why Object::<init> doesn't get compiled to C2 and propose the improvement here to be done with the existing Method->is_empty_method that will leave the special Object::<init> method alone.
26-04-2018

Please see JDK-8145579 for benchmark that shows regression when register finalizer is done by C1 instead of C2. So you should see a regression unless the C1 intrinsic is improved.
26-04-2018

Good!
26-04-2018

Testing with a patch that includes Object::<init> in the is_trivial predicate we see the same 1 -> deopt -> 1 transition
26-04-2018

Yes. Experimentally we see that Object::<init> is compiled to level 3, then to 1, then to 1 again: $ 10/bin/java -XX:+PrintCompilation Hello | grep "Object::<init" 54 2 3 java.lang.Object::<init> (1 bytes) 64 26 1 java.lang.Object::<init> (1 bytes) 65 2 3 java.lang.Object::<init> (1 bytes) made not entrant 116 26 1 java.lang.Object::<init> (1 bytes) made not entrant 120 125 1 java.lang.Object::<init> (1 bytes) -XX:+TraceDependencies (thanks @neliasso) shows the last deopt+recompilation is due to a change in the no_finalizable_subclasses dependency for Object::<init>: Failed dependency of type no_finalizable_subclasses context = java.lang.Object witness = java.io.FileOutputStream$AltFinalizer code: nmethod 505 35 1 java.lang.Object::<init> (1 bytes) Marked for deoptimization ...
26-04-2018

Are you sure that level 1 code will be deoptimized?
26-04-2018

Right, seems this is done by and explained in rewriter.cpp: // The new finalization semantics says that registration of // finalizable objects must be performed on successful return from the // Object.<init> constructor. We could implement this trivially if // <init> were never rewritten but since JVMTI allows this to occur, a // more complicated solution is required. A special return bytecode // is used only by Object.<init> to signal the finalization // registration point. Additionally local 0 must be preserved so it's // available to pass to the registration function. For simplicity we // require that local 0 is never overwritten so it's available as an // argument for registration. void Rewriter::rewrite_Object_init(const methodHandle& method, TRAPS) It seems to me that C1 is indifferent to this bytecode rewriting scheme - the level 1 compiled Object.<init> will deopt and be recompiled once a finalizable class is loaded - so treating Object.<init> as a trivial method seems like it should be fine.
26-04-2018

Coleen wrote: we rewrite the return in Object.<init> function into return_register_finalizer in the rewriter so that we can register the finalizer after Object.<init> would logically be executed
26-04-2018

Claes wrote: seems any <init> uses the same return as a return from any void method: public java.lang.Object(); descriptor: ()V flags: (0x0001) ACC_PUBLIC Code: stack=0, locals=1, args_size=1 0: return LineNumberTable: line 50: 0 LocalVariableTable: Start Length Slot Name Signature 0 1 0 this Ljava/lang/Object; RuntimeVisibleAnnotations: 0: #27() jdk.internal.HotSpotIntrinsicCandidate any *other* "empty" <init> except Object.<init> will look something like this: public E(); descriptor: ()V flags: ACC_PUBLIC Code: stack=1, locals=1, args_size=1 0: aload_0 1: invokespecial #15 // Method java/lang/Object."<init>":()V 4: return LineNumberTable: line 20: 0
26-04-2018

What bytcodes Object.<init> has?
26-04-2018