Re: RFR: 8218732: SA: Resolves ZPageAllocator::_physical incorrectly

2019-02-14 Thread Stefan Karlsson 

Thanks, Erik.

Stefank

On 2019-02-14 09:26, Erik Österlund wrote:

Hi Stefan,

Given that the remark from Jini is fixed, this looks good. I don't need 
another webrev.


Thanks,
/Erik

On 2019-02-11 19:09, Stefan Karlsson wrote:

Hi Jini,

On 2019-02-11 19:00, Jini George wrote:

Hi Stefan,

Looks good to me overall. One point though.

physicalFieldOffset = type.getAddressField("_physical").getOffset();

Wrt the line above, is there any reason due to which 
getAddressField() instead of getField() was used ? getAddressField() 
is typically used when the field to be read in is a pointer. I feel 
getField() might be more appropriate in this situation.


No, this is just an oversight. I have the same issue in my later 
OopHandle patch. I'll change this.


Thanks for reviewing,
StefanK



Thanks,
Jini.

On 2/11/2019 1:23 PM, Stefan Karlsson wrote:

Hi all,

Please review this small patch to resolve ZPageAllocator::_physical 
as a value object instead of a pointer.


https://cr.openjdk.java.net/~stefank/8218732/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8218732

This fixes the Heap Parameters view.

Thanks,
StefanK

Re: RFR: 8218732: SA: Resolves ZPageAllocator::_physical incorrectly

2019-02-14 Thread Erik Österlund 

Hi Stefan,

Given that the remark from Jini is fixed, this looks good. I don't need 
another webrev.


Thanks,
/Erik

On 2019-02-11 19:09, Stefan Karlsson wrote:

Hi Jini,

On 2019-02-11 19:00, Jini George wrote:

Hi Stefan,

Looks good to me overall. One point though.

physicalFieldOffset = type.getAddressField("_physical").getOffset();

Wrt the line above, is there any reason due to which getAddressField() 
instead of getField() was used ? getAddressField() is typically used 
when the field to be read in is a pointer. I feel getField() might be 
more appropriate in this situation.


No, this is just an oversight. I have the same issue in my later 
OopHandle patch. I'll change this.


Thanks for reviewing,
StefanK



Thanks,
Jini.

On 2/11/2019 1:23 PM, Stefan Karlsson wrote:

Hi all,

Please review this small patch to resolve ZPageAllocator::_physical 
as a value object instead of a pointer.


https://cr.openjdk.java.net/~stefank/8218732/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8218732

This fixes the Heap Parameters view.

Thanks,
StefanK

Re: RFR: 8218732: SA: Resolves ZPageAllocator::_physical incorrectly

2019-02-11 Thread Stefan Karlsson 

Hi Jini,

On 2019-02-11 19:00, Jini George wrote:

Hi Stefan,

Looks good to me overall. One point though.

physicalFieldOffset = type.getAddressField("_physical").getOffset();

Wrt the line above, is there any reason due to which getAddressField() 
instead of getField() was used ? getAddressField() is typically used 
when the field to be read in is a pointer. I feel getField() might be 
more appropriate in this situation.


No, this is just an oversight. I have the same issue in my later 
OopHandle patch. I'll change this.


Thanks for reviewing,
StefanK



Thanks,
Jini.

On 2/11/2019 1:23 PM, Stefan Karlsson wrote:

Hi all,

Please review this small patch to resolve ZPageAllocator::_physical 
as a value object instead of a pointer.


https://cr.openjdk.java.net/~stefank/8218732/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8218732

This fixes the Heap Parameters view.

Thanks,
StefanK

Re: RFR: 8218732: SA: Resolves ZPageAllocator::_physical incorrectly

2019-02-11 Thread Jini George 

Hi Stefan,

Looks good to me overall. One point though.

physicalFieldOffset = type.getAddressField("_physical").getOffset();

Wrt the line above, is there any reason due to which getAddressField() 
instead of getField() was used ? getAddressField() is typically used 
when the field to be read in is a pointer. I feel getField() might be 
more appropriate in this situation.


Thanks,
Jini.

On 2/11/2019 1:23 PM, Stefan Karlsson wrote:

Hi all,

Please review this small patch to resolve ZPageAllocator::_physical as a 
value object instead of a pointer.


https://cr.openjdk.java.net/~stefank/8218732/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8218732

This fixes the Heap Parameters view.

Thanks,
StefanK