RFR(xs): [windows] small fixes to os::check_heap()

David Holmes david.holmes at oracle.com
Thu Nov 19 07:49:21 UTC 2015


Hi Thomas,

On 19/11/2015 4:52 PM, Thomas Stüfe wrote:
> Hi David,
>
> On Thu, Nov 19, 2015 at 4:56 AM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
>     Hi Thomas,
>
>     On 19/11/2015 2:40 AM, Thomas Stüfe wrote:
>
>         Hi all,
>
>         could you please sponsor and review these tiny changes to
>         os::check_heap()
>         on windows:
>
>         webrev:
>         http://cr.openjdk.java.net/~stuefe/webrevs/8143233/webrev.00/webrev/
>         bug: https://bugs.openjdk.java.net/browse/JDK-8143233?filter=22798
>
>         Fixes two issues:
>         - GetProcessHeap() is not necessarily the CRT heap, so we need
>         to use
>         _get_heap_handle(); instead
>
>
>     I'm unclear why we want the CRT heap as opposed to the process heap?
>
>
> os::check_heap is used in a context that lets me assume that the CRT
> heap is meant (e.g. in os::malloc, os::realloc, triggered by
> MallocVerifyInterval
> <http://ld8443:8080/source/s?defs=MallocVerifyInterval&project=integ-hotspot>).
> Also, we do not explicitly use the Process Default Heap, so there is no
> use in checking it.
>
>     But in any case according to this as of VS2012 the CRT heap and
>     process default heap are the same:
>
>     http://stackoverflow.com/questions/18955871/why-does-get-heap-handle-equal-to-getprocessheap
>
>
> True, but
> 1) we still support VS 2010
> (https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms)

Wasn't aware of that :)

> 2) arguably, use of _get_heap_handle() better documents the intent here
> 3) Microsoft may decide at some point to switch back to a separate heap
> for the CRT instead of using the process default heap.

Okay. Only issue I have is actually testing the change. I don't see that 
we currently have any jtreg tests that exercise this heap checking :( 
Did I miss something?

Thanks,
David


> Regards, Thomas
>
>         - We need to always unlock the heap before exiting with fatal()
>         in case we
>         found an error.
>
>
>     Ok.
>
>     Thanks,
>     David
>
>         Kind Regards, Thomas
>
>


More information about the hotspot-runtime-dev mailing list