[JFR: jdk8u-jfr-incubator] RFR: JDK-8238589: Necessary code cleanup in JFR for JDK8u
Mario Torre
neugens at redhat.com
Wed Feb 12 15:59:33 UTC 2020
On Wed, Feb 12, 2020 at 4:37 PM Aleksey Shipilev <shade at redhat.com> wrote:
>
> On 2/12/20 1:42 PM, Mario Torre wrote:
> >> *) In systemDictionary.cpp, this assert seems legit, no need to remove it? It guards from calling
> >> the branch when jfr_event_handler_proxy is accidentally uninitialized:
> >>
> >> 1342 assert(jfr_event_handler_proxy != NULL, "invariant");
> >
> > But if jfr_event_handler_proxy is null then also class_name needs to
> > be null for the code to be executed, I don't think this is possible?
>
> I don't want these two things to interact. So this is why you need the assert before the branch to
> verify that we are testing against the sane value:
>
> #if INCLUDE_JFR
> assert(jfr_event_handler_proxy != NULL, "invariant");
> if (k.is_null() && (class_name == jfr_event_handler_proxy)) {
> ...
I'm a bit afraid that now the assert will be executed no matter what.
Before it would fall inside an "else" so only execute if k.is_null.
Am I seeing this wrong?
Cheers,
Mario
> > http://cr.openjdk.java.net/~neugens/8238589/webrev.02/
>
> Otherwise looks fine.
>
> Stylistically, I don't think you need the comment at #endif of short blocks like:
>
> 259 #if INCLUDE_JFR
> 260 // If requested, return information on which thread held the bias
> 261 if (biased_locker != NULL) {
> 262 *biased_locker = biased_thread;
> 263 }
> 264 #endif // INCLUDE_JFR
>
> ...as it is dead obvious what that #endif belongs to.
>
> --
> Thanks,
> -Aleksey
>
--
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30 9205 5D7E 4952 3F65 7898
More information about the jdk8u-dev
mailing list