Discussion about fixing deprecation in jdk.hotspot.agent

[Chris Plummer]

On 3/31/20 1:32 PM, coleen.phillimore at oracle.com wrote:
>
>
> 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.
I'm trying to understand exactly when all these static initializes are 
triggered. Is it only after you do an attach?

The implementation of clhsdb reattach is exactly the same as doing a 
detach followed by an attach to the same process. I'm not sure how much 
value it has, but I think in general the removal of this code means you 
can't detach and then attach to anything, even a different pid. So 
"detach" might as well become "detach-and-exit", because your clhsdb 
session is dead once you detach. Do we really want to do this?

Chris
>
> 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
>>>>>
>>>>
>>>
>>
>