RFR: 8266074: Vtable-based CHA implementation

[David Holmes]

On 2/05/2021 2:00 pm, Igor Ignatev wrote:
> Hi David,
> 
> I meant both: in a generic sense, meaning we won't properly document, 
> advertise it or claim it as supported; and changing its type to 
> EXPERIMENTAL, so it will be somewhat harder for people to switch it.

Both forms are as hard to switch to as both must be unlocked. But 
semantically this is not an experimental flag IMO.

Regardless, if the intent is to allow the flag to be used to restore 
legacy behaviour, then that legacy behaviour must be tested. We don't 
have to run thousands of tests with the flag disabled, just something 
representative enough to give us confidence that the code has not 
bit-rotted.

Cheers,
David

> -- Igor
> 
>> On May 1, 2021, at 3:03 PM, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> On 2/05/2021 1:39 am, Igor Ignatyev wrote:
>>> On Fri, 30 Apr 2021 22:10:02 GMT, Vladimir Kozlov <kvn at openjdk.org 
>>> <mailto:kvn at openjdk.org>> wrote:
>>>>> I'm fine with both approaches.
>>>>>
>>>>> Explicitly setting the flag looked to me more robust and clearer 
>>>>> communicating the intent. But if you prefer `@requires`, I'll use it.
>>>>
>>>> Let hear @iignatev opinion.
>>> from my point of view, `@requires` is clearer and also eliminates 
>>> "wasted" execution (if someone tries to run this test w/ 
>>> `-XX:-UseVtableBasedCHA`), so I'd prefer if we use it.
>>> I have a more generic comment about `UseVtableBasedCHA`. I understand 
>>> the desire to introduce a flag to switch back to the old 
>>> implementation, but I'm somewhat concern that it adds a new dimension 
>>> into configuration space that won't be covered by our existing tests 
>>> (w/ the test which exercises interesting parts of the related code is 
>>> inapplicable) and isn't part of our regular test configurations. Can 
>>> we make it an experimental flag (w/ vtable-based CHA still being 
>>> enabled by default)? this way, the quality bar for the old 
>>> implementation will be somewhat lower, yet the end-users will still 
>>> be able to return to the old implementation if it, for some reason, 
>>> works better in their use-cases.
>>
>> Did you mean "experimental" in a generic sense or actually change it 
>> from DIAGNOSTIC to EXPERIMENTAL? If the latter then I don't agree this 
>> is an experimental flag, it is diagnostic. But either way the testing 
>> requirements are the same if we expect to tell end users to try this 
>> flag if they hit an problem - the flag has to be known to be 
>> functional, so we will have to expand the test coverage.
>>
>> Cheers,
>> David
>> -----
>>
>>> -- Igor
>>> -------------
>>> PR:https://git.openjdk.java.net/jdk/pull/3727 
>>> <https://git.openjdk.java.net/jdk/pull/3727>
>