[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: Discussion about fixing deprecation in jdk.hotspot.agent
From:       coleen.phillimore () oracle ! com
Date:       2020-03-31 21:20:19
Message-ID: 1cc556ce-67ed-1e6f-ee53-36d8227d0e1e () oracle ! com
[Download RAW message or body]



On 3/31/20 4:55 PM, Chris Plummer wrote:
> On 3/31/20 1:32 PM, coleen.phillimore@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?

Well, that was my question. It seems like you could just exit and start 
up jhsdb again and that's more like something someone would do just as 
easily.  Given the use cases that we've seen from sustaining, this 
appears to be unneeded functionality.

The original mail was proposing adding more code to work around the 
deprecation messages.  It seems like more code should not be added for 
something that is unused.

thanks,
Coleen

>
> Chris
>>
>> Thanks,
>> Coleen
>>>
>>> Thanks,
>>> Poonam
>>>
>>> On 3/31/20 5:34 AM, coleen.phillimore@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@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
>>>>>>
>>>>>
>>>>
>>>
>>
>
>

[prev in list] [next in list] [prev in thread] [next in thread]