That was the idea. However, can I have Ok for checking this into hs24 while waiting?
Thanks /R On Jan 21, 2013, at 11:33 PM, David Holmes wrote: > On 22/01/2013 12:09 AM, Rickard Bäckman wrote: >> Yes, that code has changed. Checked in to hs24. > > Okay but this is a review for hs25 ;-) So I assume that change will be there > "real soon now". :) > > David > >> /R >> >> 21 jan 2013 kl. 02:59 skrev David Holmes<david.hol...@oracle.com>: >> >>> On 18/01/2013 11:45 PM, Rickard Bäckman wrote: >>>> Aleksey, >>>> >>>> thanks for your review! >>>> >>>> a) It was before on of my own changes used in os_solaris.cpp (I think, for >>>> synchronization support for Suspend/Resume). >>>> I don't think we wanted something external to mess with that lock. >>> >>> Seems to be used here: >>> >>> ./os/solaris/vm/os_solaris.cpp: >>> >>> 4265 GetThreadPC_Callback cb(ProfileVM_lock); >>> >>> Is this code already undergoing removal as part of the JFR changes? >>> >>> Thanks, >>> David >>> ----- >>> >>> >>>> b) I've changed the indentation slightly. >>>> Updated webrev at http://cr.openjdk.java.net/~rbackman/8006563.2/ (or at >>>> least currently copying…) >>>> >>>> /R >>>> >>>> On Jan 18, 2013, at 2:12 PM, Aleksey Shipilev wrote: >>>> >>>>> On 01/18/2013 04:58 PM, Rickard Bäckman wrote: >>>>>> http://cr.openjdk.java.net/~rbackman/8006563/ >>>>> >>>>> Looks good to me (not a Reviewer), modulo: >>>>> a) Are we sure this thing is not acquired in some weird way, i.e. >>>>> through JVMTI, SA, or whatnot? >>>>> b) The formatting of the predicate does not follow it's structure, I >>>>> think it should be: >>>>> ... >>>>> this != Interrupt_lock&& >>>>> !(this == Safepoint_lock&& >>>>> contains(locks, Terminator_lock)&& >>>>> SafepointSynchronize::is_synchronizing())) { >>>>> >>>>> This way it is more obvious SafepointSynchronize::is_synchronizing()) is >>>>> the !(...) group. >>>>> >>>>> -Aleksey. >>>>
