On 7/20/20 10:47 PM, David Holmes wrote:
Hi Coleen,
cc'ing serviceability due to SA changes.
On 21/07/2020 6:53 am, coleen.phillim...@oracle.com wrote:
Summary: Move static oops into OopStorage and make NPE oops an
objArrayOop.
I've broken up moving oops in Universe to OopStorage into several
parts. This change moves the global static oops. These OopHandles
are not released.
Overall looks good. But two things ...
1. naming
! // preallocated error objects (no backtrace)
! static OopHandle _out_of_memory_error;
// array of preallocated error objects with backtrace
! static OopHandle _preallocated_out_of_memory_error_array;
Both of these are pre-allocated arrays of OopHandles, differing only
in whether the underlying OOME has a backtrace or not. I find the
newly introduced _out_of_memory_error unclear in that regard. At a
minimum could _out_of_memory_error become _out_of_memory_errors ? But
ideally can we name these two arrays in a more distinguishable way?
I put this code in functions next to each other because it was
confusing. The _out_of_memory_error array is really preallocated
throwables, that are used to fill in the message for
preallocated_out_of_memory_errors if there are enough of the latter left.
I could rename _out_of_memory_error => _out_of_memory_error_throwables ?
2. SA
You've simply deleted all the SA/vmstructs code that referenced the
oops that are no longer present. Does the SA not care about these
things and need access to them? (I don't know hence cc to
serviceability folk.)
Yes, the SA code was never used, so I deleted it. SA might need in oop
inspection to add walking Universe::vm_global() OopStorage area to find
where oops come from if there's an error but there doesn't seem to be
any other reason for SA to use these oops.
Thanks,
Coleen
Thanks,
David
-----
This has been tested with tier1-3 and on also remote-build -b
linux-arm32,linux-ppc64le-debug,linux-s390x-debug,linux-x64-zero.
open webrev at
http://cr.openjdk.java.net/~coleenp/2020/8249768.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8249768
Thanks,
Coleen
