JDK-8147978 : Remove Method::_method_data for C1
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 9
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2016-01-21
  • Updated: 2016-03-15
  • Resolved: 2016-03-01
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.
JDK 9
9 b110Fixed
Related Reports
Cloners :  
Relates :  
Relates :  
Description
This is item #8 from JDK-8138571. Method::_method_data appears to only be needed for C2. It can be removed when only C1 is supported to save footprint.
Comments
Since there are clear C1 dependencies on the mdo, I've decided that it's best not to try to implement this change. It's likely possible to remove these dependencies, but that would hurt C1 performance. They could also just be removed for minimal VM builds, but that would make the C1 you get in minimal VM different from the one you get in client VM, which would reduce test coverage. It just seems this footprint opt is not worth the effort.
11-03-2016

Disabling RangeCheckElimination and UseLoopInvariantCodeMotion makes the problem go away by causing is_optimistic() to return false. However, just disabling UseLoopInvariantCodeMotion also makes the problem go away by getting rid of the optimization that is resulting in the predicate check being generated.
10-03-2016

The deopt is being triggered by a PredicateFailedStub at the start of the loop in CharArrTest::compareAsCharArrays. I'm not sure what the predicate is detecting. The code below is the c1 user of the mdo. Note that both RangeCheckElimination and UseLoopInvariantCodeMotion are enabled. If I disable both, the problem goes away. // will compilation make optimistic assumptions that might lead to // deoptimization and that the runtime will account for? bool is_optimistic() const { return !TieredCompilation && (RangeCheckElimination || UseLoopInvariantCodeMotion) && method()->method_data()->trap_count(Deoptimization::Reason_none) == 0; } And the above code is called from: void GraphBuilder::if_node(Value x, If::Condition cond, Value y, ValueStack* state_before) { ... // In case of loop invariant code motion or predicate insertion // before the body of a loop the state is needed Instruction *i = append(new If(x, cond, false, y, tsux, fsux, (is_bb || compilation()->is_optimistic()) ? state_before : NULL, is_bb));
10-03-2016

I've narrowed this down a bit. Without this change in place, I see CharArrTest::compareAsCharArrays get compiled, deopt, and then compiled a second time. This test completes shortly after the second compilation. With this change in place, this method gets repeatedly compiled and deopt until eventually the test times out. I also narrowed down that the only change needed to cause the problem is the change in c1_Runtime1.cpp that makes it so it only create the mdo if ProfileInterpreter is true.
08-03-2016

Fix failed: JDK-8151130
03-03-2016

Today, no, but it might in the future. Note: INCLUDE_JVMCI is not a define; it's either 0 or 1.
22-01-2016

Will INCLUDE_JVMCI ever be defined when only C1 is built?
22-01-2016

Wherever there is a if defined(COMPILER2) we also need a INCLUDE_JVMCI. JVMCI compilers need profiling information.
22-01-2016

The patch from JDK-8138571 applied cleanly (after removing a bunch of changes unrelated Method::_method_data removal) and passed jprt hotspot testing. However, there are some issues to resolve: 1. It crashes when using -XX:+ProfileInterpreter with C1. It looks like there is potentially a lot of ProfileInterpreter code that depends on Method::_method_data. I'm not sure if the correct fix is to force ProfileInterpreter=false for C1. Are there performance consequences for doing this? If so, we may need make it so Method::_method_data is only removed for minimal VM. 2. It's unclear to me why the change in c1_Runtime1.cpp added "if (ProfileInterpreter)" rather than "#ifdef COMPILER2" 3. Are there potential performance issues to removing Method::_method_data when ProfileInterpreter=false, or it it really not needed when ProfileInterpreter=false?
21-01-2016