Thanks for reviews, Jim and Mandy! I've pushed the change...
On 03/02/2017 12:06 AM, Jim Laskey (Oracle) wrote:
My only concern at this point is if Claes and I get hit by a bus, no one will
be able to figure this out. I recommend that next release that we switch to
character based hash. This means some minor complexity in the C code but I
think the java code will be much simpler (and the C not so bad.)
I agree that it'd be interesting to explore this in the future.
/Claes
On Mar 1, 2017, at 7:42 PM, Mandy Chung <mandy.ch...@oracle.com> wrote:
On Feb 27, 2017, at 6:55 PM, Claes Redestad <claes.redes...@oracle.com> wrote:
Hi,
thanks Mandy and Jim for reviewing!
However, I've found enough evidence now that we should this
one step further and eliminating the allocation in
BasicImageReader::findLocation(String, String), which completely
gets rid of the regressions we're seeing:
http://cr.openjdk.java.net/~redestad/8175561/jdk.02/
http://cr.openjdk.java.net/~redestad/8175561/jdk.01.02.diff/
This looks correct but we need Jim to confirm the bug you spotted
ImageLocationWriter.
59 public static int hashCode(String string) {
and all of other hashCode methods.
Nit: "String name" should work as it matches the parameter in the caller
method. Or `s` might be better than “String string”.
153 static boolean verify(String module, String name,
154 final long[] attributes, final ImageStrings strings) {
67 private static boolean verifyName(String name, int index, final int
length,
168 final long[] attributes, final ImageStrings strings) {
Nit: some final and some non-final parameters and better to be consistent. Any
reason why you mark it “final”?
No need for a new webrev.
Thanks
Mandy
