Discussion about fixing deprecation in jdk.hotspot.agent

[coleen.phillimore at oracle.com]

On 3/31/20 12:19 PM, Poonam Parhar wrote:
> 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.

Hi Poonam,  Thank you for answering. Yes, this patch only removes the 
reattach functionality.  I tried out the other clhsdb commands from your 
wiki page, and they worked fine, including object and heap inspection.

Thanks,
Coleen
>
> 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
>>>>
>>>
>>
>