Hi,
Also including build-dev since this touches some makefiles.
Thanks Dan! see inline, also new webrev:
http://cr.openjdk.java.net/~jfranck/8009382/hotspot/webrev.02/
On 03/25/2013 04:17 PM, Daniel D. Daugherty wrote:
On 3/22/13 8:16 AM, Joel Borggrén-Franck wrote:
make/bsd/makefiles/mapfile-vers-debug
make/linux/makefiles/mapfile-vers-debug
make/linux/makefiles/mapfile-vers-product
make/solaris/makefiles/mapfile-vers
It looks like the other entries are in alpha order so these
two new entries should also be added in alpha order.
Fixed. I also noticed I forgot to update one of the bsd makefiles.
src/share/vm/prims/jvm.cpp
line 1510: return NULL;
nit: indent should be two spaces
line 1529: assert(false, "cannot find method");
line 1530: return NULL; // robustness
Normally "robustness" comments flag logic after an assert()
to indicate what we do in product mode to deal with the "bad"
situation without crashing. In this case, we don't try to use
'm' after discovering that it is NULL so this could be simpler:
Method* m = InstanceKlass::cast(k)->method_with_idnum(slot);
assert(m != NULL, "cannot find method");
return m; // caller has to deal with NULL in product mode
Yes, I realize you only touched the comment here, but it
served to point out the messiness of the existing code.
Good suggestion. Fixed.
line 1551: return NULL;
line 1565: return NULL;
line 1579: return NULL;
nit: indent should be two spaces
line 1632: return NULL;
line 1613: return NULL;
nit: indent should be two spaces
Fixed all indent nits.
A prototype of the jdk changes can be found here:
http://cr.openjdk.java.net/~jfranck/8009382/jdk/webrev.00/
You should use a separate bug ID for the jdk repo changes. This will
prevent confusion between when HSX with these changes is integrated
into JDK8 and when the jdk repo changes are integrated into JDK8.
The JDK changes have a separate bug, sorry if this was unclear. The jdk
changes shown are just a proof of concept, intended to highlight that my
HotSpot changes have actually been run. I will send out a separate review
request in the future for the JDK changes.
cheers
/Joel
