Question re -XX:-FlightRecorder

[Erik Gahlin]

> On 7 Oct 2020, at 10:38, Andrey Petushkov <andrey.petushkov at gmail.com> wrote:
> 
> Hi Erik,
> 
> On 06.10.2020 18:32, Erik Gahlin wrote:
>> Hi Andrey,
>> 
>> The flag is legacy from JDK 7 when we needed -XX:+FlightRecorder to allocate global buffers when the VM started. That restriction was removed in 8u40. it is probably a good idea to remove the flag completely. The method JfrRecorder::is_disabled() should be removed at the same time.
> The flag is marked as deprecated so can I just remove it or should it be
> promoted to the next stage (obsolete)?

Make it obsolete.

>> When it comes to earlier releases, I think we need to stick to the behaviour that exists. Changing it in an update release would cause even more confusion. 
> Well, I assume you don't mean VM crash on assert is the behavior you'd
> like to retain ;)

When removing JfrRecorder::is_disabled(), also delete the assert.

> So I read this as:
> - FlightRecorder.isAvailable to return false
> - FlightRecorder.getFlightRecorder, new Recording() etc are to work as
> if no -XX:-FlightRecorder was specified

If this was the behaviour in earlier releases, I think we should keep it. It is inconsistent, but changing either of these, could lead to regressions when somebody gets a security upgrade. If there was a good use case for -XX:-FlightRecorder, one could argue for a change, but then we shouldn’t remove the flag in the first place.

Thanks
Erik

>> Do you know why users want to use -XX:-FlightRecorder?
> Not really. It might be that this were mostly related to JFR backport
> into 8u and the customers were cautious about it's performance impact
>> 
>> Thanks
>> Erik
> Thank you,
> Andrey
>> 
>>> On 6 Oct 2020, at 11:44, Andrey Petushkov <andrey.petushkov at gmail.com> wrote:
>>> 
>>> Hi Erik, All,
>>> 
>>> sometimes ([1], [2]) the message is given that the command line switch
>>> -XX:-FlightRecorder disables the JFR functionality. The problem is
>>> that this statement is not true and in fact causes buggy behavior of
>>> the VM. Namely:
>>> - on a release VM ran with -XX:-FlightRecorder the call to
>>> FlightRecorder.getFlightRecorder() succeeds and the recording can be
>>> successfully created (much like as if no command line flag was given)
>>> - on a debug version of VM ran with same flag the call to
>>> FlightRecorder.getFlightRecorder() crashes the VM on assert
>>> #  Internal Error
>>> (/home/andrey/ws/jdk/src/hotspot/share/jfr/recorder/jfrRecorder.cpp:236),
>>> pid=331136, tid=331137
>>> #  assert(!is_disabled()) failed: invariant
>>> 
>>> It would be good to clarify the behavior and implement it in the
>>> consistent way, that is, either:
>>> - allow -XX:-FlightRecorder to actually disable JFR forever, and throw
>>> IllegalStateException from FlightRecorder.getFlightRecorder() and
>>> friends
>>> - declare -XX:-FlightRecorder as no-op and remove the mentioned assert
>>> Please note that the answer might be different for pre-13 and 13+
>>> versions because of flag deprecation
>>> 
>>> Any opinion on this?
>>> This might not look like an important thing at all however there were
>>> quite a number of users (Azul customers) stumbling on unclear behavior
>>> of the said flag so I thought it's better to have this fixed
>>> 
>>> Thanks a lot,
>>> Andrey
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8225312
>>> [2] jdk/src/hotspot/share/jfr/recorder/jfrRecorder.cpp,
>>> JfrRecorder::is_disabled()
>