[9] RFR(S): 8059606: Enable per-method usage of CompileThresholdScaling (per-method compilation thresholds)

[Zoltán Majó]

Hi John,


On 01/05/2015 09:00 PM, John Rose wrote:
> On Jan 5, 2015, at 11:13 AM, Vladimir Kozlov 
> <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>
>> On 1/5/15 10:38 AM, Zoltán Majó wrote:
>>> ...
>>>
>>> Per-method compilation thresholds are available only in non-product 
>>> builds to avoid the overhead of accessing fields
>>> added by the patch MethodData and MethodCounters.
>>
>> Too many ifdefs :)
>> The interpreter speed is not important. And the feature could be 
>> interesting in product VM too.
>> The only drawback is 2 additional fields in MDO which is fine.
>> Can you make it product and run through our performance infrastructure.
>> Also, as John Rose will say, we should have as much as possible a 
>> similar code in product as in tested debug code. Otherwise we are not 
>> testing product bits and will get into troubles.
>
> Vladimir is right; please consider it said by both of us.  :-)

thank you for the review!

> Zoltan, I am glad you are tackling the profiling code, because we need 
> experts in it.

Then I'm glad I've touched this part of the code.

>
> In the long run, the invocation counter code started complex and is 
> growing more complex.  We need to rationalize the counters more.  The 
> notification frequency idea is a good step in the right direction, 
> since it pulls logic out of the assembly code and into high-level 
> event handling code.  Another good step would be to reduce the number 
> of distinct counters visible at the assembly level, thus simplifying 
> the assembly code.  The complexity of InvocationCounter is a fossil 
> which deserves to be buried and paved over.

Currently, the VM uses runtime notifications only with tiered 
compilation enabled. But runtime notification could be used in the 
non-tiered part of the code as well.

>
> Speaking of ifdefs, I am uncomfortable with the ifdef-TIERED branches 
> in the assembly code.  Can we move towards merging those code branches?

If the VM is changed to use notification frequencies also with tiered 
compilation disabled, I think it won't be difficult to merge those two 
code paths.

>
> And, speaking of footprint, I see no reason why we couldn't shrink the 
> Method layout to handle all counter bookkeeping with a single word, 
> instead of the current two.  Also (while I'm on the subject) 
> low-count, non-looping methods do not need a full MethodCounter 
> struct, just a simple inline count with a low-tag bit, with CAS-based 
> state changes.  This would have the effect of delaying MC and MD 
> allocation  until a method has been used non-trivial amount, which 
> would reduce footprint if "non-trivial amount" turns out to be large.
>
>    counters = union {
>      uintptr_t simple_count;  // c = (invocation_count << 16 | 
> notify_mask << 8 | other_flags_we_might_like << 1 | 1)
>      uintptr_t method_counters;  // c = 
> ((intptr_t)method_counters_addr | 0)
>      uintptr_t method_data;  // c = ((intptr_t)method_data_addr | 0)
>    }
>
> (Assumes that method_data and method_counters can be distinguished 
> suitably by their contents.)

That is also a good idea.

In general, I agree that profiling infrastructure in the interpreter is 
too complex. I also think your ideas could simplify the profiling 
infrastructure quite a lot. So I opened a separate issue, 8068667 -- 
"simplify interpreter profiling infrastructure", that is dedicated to 
reducing the complexity of interpreter code:

https://bugs.openjdk.java.net/browse/JDK-8068667

I hope I can take care of it soon.

Thank you!

Best regards,


Zoltan

> — John