On 2020-04-14 14:44, [email protected] wrote:
This looks good to me since we can't remove this.
Thanks for your review!
Does jdk.hotspot.agent follow the Hotspot review rules requiring two
Reviewers?
The patch complains that for your Observer.java:
\ No newline at end of file
Yeah, I just noticed that. :-( I'll fix that before pushing.
/Magnus
Thanks,
Coleen
On 4/14/20 7:04 AM, Magnus Ihse Bursie wrote:
As a first step towards fixing deprecation warnings in SA, all the
references (200+) to the deprecated java.util.Observer and Observable
needs to be fixed, otherwise all other changes will drown in this one.
This solution is the result of the preceding discussions in
serviceability-dev. That means that most of the change consists of
adding an explicit import like this:
import sun.jvm.hotspot.utilities.Observable;
import sun.jvm.hotspot.utilities.Observer;
to override the general java.util.* import that was already present
in all (or almost all) files, and make
sun.jvm.hotspot.utilities.Observable and Observer extend the
java.util versions but with deprecation warnings disabled.
It turned out however, that this simplest approach did not work
fully. Since the interface java.util.Observer had the single method
"void update(java.util.Observable o, Object arg)" it did not help to
create a new interface sun.jvm.hotspot.utilities.Observer that
extended java.util.Observer. I did not observe this issue in my PoC
webrev that I posted during the discussion. :-(
Instead, for Observer, I had just created a new interface with the
same method, but that uses sun.jvm.hotspot.utilities.Observable
instead of java.util.Observable.
The end effect is the same -- the only change needed to most files is
an added import, we get rid of the deprecation warnings, and we did
not have to copy any significant amount of code from java.util.
I now hope this is acceptable by all.
Bug: https://bugs.openjdk.java.net/browse/JDK-8242629
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8242629-fix-SA-Observer/webrev.01
(When reading the patch, I recommend looking at the patch file
http://cr.openjdk.java.net/~ihse/JDK-8242629-fix-SA-Observer/webrev.01/open.patch
instead of individually checking the files in the webrev.)
/Magnus
