Modified webrev:
http://cr.openjdk.java.net/~jgeorge/8171084/webrev.01/index.html
<http://cr.openjdk.java.net/%7Ejgeorge/8171084/webrev.01/index.html>
Requesting one more Reviewer also to take a look at this.
Thank you,
Jini.
On 1/23/2017 2:10 PM, Jini George wrote:
Thanks much for the review, David. My answers inline:
870 protected void writeInstance(Instance instance, int length)
throws IOException {
seems incorrect. It adds a length parameter that is unused and also
creates an overload, rather than override of the inherited version.
As a result this code:
Thank you for this very good catch! This was an accidental cut-n-paste
error. I have corrected this.
Minor nit:
379 private static final long MAX_U4_VALUE = 4294967295L;
would be clearer as:
379 private static final long MAX_U4_VALUE = 0xFFFFFFFFL;
Makes sense. Done.
test/serviceability/sa/LingeredAppWithLargeArray.java
Style nit:
27 public int hugeArray[];
should be
27 public int[] hugeArray;
but why public ??
No particular reason. Changed it.
I don't know how LingeredApp works, but this looks odd:
32 public static void main(String args[]) {
33 LingeredAppWithLargeArray appObject = new
LingeredAppWithLargeArray();
34 LingeredApp.main(args);
35 }
as the appObject is never used. ??
I have changed the test case now to have the array allocation done in
main() and not in the constructor.
test/serviceability/sa/TestHeapDumpForLargeArray.java
Why is the test excluded on Mac?
There used to be SA related failures on Mac a while back, but it seems
those have gotten fixed with 8169638
<https://bugs.openjdk.java.net/browse/JDK-8169638> and probably
8160376 <https://bugs.openjdk.java.net/browse/JDK-8160376>. I am
removing the restriction and will be running the tests. Planning on
sending out a new webrev once the testing comes clean.
Thanks,
Jini.
