Discussion about fixing deprecation in jdk.hotspot.agent

[Poonam Parhar]

Hello Coleen,

Does the removal of this code only impact the 'reattach' functionality, 
and it does not affect any commands available in 'clhsdb' once it is 
attached to a core file? If that's true, then I think it should be okay 
to remove this code.

Thanks,
Poonam

On 3/31/20 5:34 AM, coleen.phillimore at oracle.com wrote:
>
> To answer my own question, this functionality is used to allow 
> detach/reattach from {cl}hsdb.  Which seems to work on linux but not 
> windows with this code removed.
>
> The next question is whether this is useful functionality to justify 
> all this code (900+ and this new code that Magnus has added).  Can't 
> you just exit and restart the clhsdb process on the core file or process?
>
> For the record, this is me playing with python to remove this code.
>
> http://cr.openjdk.java.net/~coleenp/2020/01/webrev/index.html
>
> Thanks,
> Coleen
>
> On 3/30/20 3:04 PM, coleen.phillimore at oracle.com wrote:
>>
>> I was wondering why this is needed when debugging a core file, which 
>> is the key thing we need the SA for:
>>
>>   /** This is used by both the debugger and any runtime system. It is
>>       the basic mechanism by which classes which mimic underlying VM
>>       functionality cause themselves to be initialized. The given
>>       observer will be notified (with arguments (null, null)) when the
>>       VM is re-initialized, as well as when it registers itself with
>>       the VM. */
>>   public static void registerVMInitializedObserver(Observer o) {
>>     vmInitializedObservers.add(o);
>>     o.update(null, null);
>>   }
>>
>> It seems like if it isn't needed, we shouldn't add these classes and 
>> remove their use.
>>
>> Coleen
>>
>> On 3/30/20 8:14 AM, Magnus Ihse Bursie wrote:
>>> No opinions on this?
>>>
>>> /Magnus
>>>
>>> On 2020-03-25 23:34, Magnus Ihse Bursie wrote:
>>>> Hi everyone,
>>>>
>>>> As a follow-up to the ongoing review for JDK-8241618, I have also 
>>>> looked at fixing the deprecation warnings in jdk.hotspot.agent. 
>>>> These fall in three broad categories:
>>>>
>>>> * Deprecation of the boxing type constructors (e.g. "new 
>>>> Integer(42)").
>>>>
>>>> * Deprecation of java.util.Observer and Observable.
>>>>
>>>> * The rest (mostly Class.newInstance(), and a few number of other 
>>>> odd deprecations)
>>>>
>>>> The first category is trivial to fix. The last category need some 
>>>> special discussion. But the overwhelming majority of deprecation 
>>>> warnings come from the use of Observer and Observable. This really 
>>>> dwarfs anything else, and needs to be handled first, otherwise it's 
>>>> hard to even spot the other issues.
>>>>
>>>> My analysis of the situation is that the deprecation of Observer 
>>>> and Observable seems a bit harsh, from the PoV of 
>>>> jdk.hotspot.agent. Sure, it might be limited, but I think it does 
>>>> exactly what is needed here. So the migration suggested in 
>>>> Observable (java.beans or java.util.concurrent) seems overkill. If 
>>>> there are genuine threading issues at play here, this assumption 
>>>> might be wrong, and then maybe going the j.u.c. route is correct.
>>>>
>>>> But if that's not, the main goal should be to stay with the current 
>>>> implementation. One way to do this is to sprinkle the code with 
>>>> @SuppressWarning. But I think a better way would be to just 
>>>> implement our own Observer and Observable. After all, the classes 
>>>> are trivial.
>>>>
>>>> I've made a mock-up of this solution, were I just copied the 
>>>> java.util.Observer and Observable, and removed the deprecation 
>>>> annotations. The only thing needed for the rest of the code is to 
>>>> make sure we import these; I've done this for three arbitrarily 
>>>> selected classes just to show what the change would typically look 
>>>> like. Here's the mock-up:
>>>>
>>>> http://cr.openjdk.java.net/~ihse/hotspot-agent-observer/webrev.01
>>>>
>>>> Let me know what you think.
>>>>
>>>> /Magnus
>>>
>>
>