VisibleMemberMap.java possible performance improvements

[Jonathan Gibbons]

On 09/23/2013 05:36 AM, Michael Simacek wrote:
> Thank you for your help, Jon.
>
> I've rewritten it a bit (also had to change the Util.executableMembersEqual method to fix some issues),
> so now all the tests should pass and I have fixed some other bugs.
> Now the order of classes is the same and order of interfaces is mostly the same.
> I tried to get as close to the original as possible, but although it seems I'm
> traversing the inheritance tree the same way as original implementation, I'm unable to retain
> the exact original order. But in most cases it is the same. What are the reasons for the
> order to stay the same?
>
> I tried to do the diff on java.lang and java.util and there are some differences, but I'm not
> sure which version is correct:
> 1. In java.lang.StringBuilder there is a section 'Methods inherited from interface CharSequence'.
>     In the original version it contains charAt, chars, codePoints, length, subSequence and in mine
>     contains just chars, codePoints. But the methods charAt, length and subSequence are overriden
>     by the StringBuilder class, so I don't think they should be there. What do you think?
>     The same thing appears in other classes, for example ArrayList.
>
> 2. AbstractQueue (and other Collections) - similar case as the above, but the methods are overriden in
>     one of the parent classes.
>
> 3. In HashMap (and any other implementations of Map) there is no MapEntry inner interface in the
>     original version, whereas in mine there is. It is public inner interface and it is not hidden
>     by anything, so I cannot see a reason why it shouldn't be there.
>
> Michael
>
>
>

Michael,

This is not so good.  What started off as a performance rewrite of one 
particular class has now expanded beyond that, as you have "fixed some 
other bugs". Even worse, the output is only "mostly the same" as before.

Here are the general guidelines ...

If you are refactoring code, such as for a performance improvement, you 
don't have to write any new tests, but the presumption is you are not 
changing the functional behavior, meaning that all existing tests should 
continue to pass, and the externally visible functionality should be 
unchanged, meaning in this case that the generated output should be the 
same (after allowing for differences like timestamps in the output.)

If you are fixing bugs, they need to identified separately, and unless 
they are very closely related, each with its own issue in 
bugs.openjdk.java.net, each one fixed and reviewed separately, and each 
with its own test(s), such as you might find in 
langtools/test/com/sun/javadoc.

JDK 8 is nearing completion. The Developer Preview has been announced 
[1].  This means we have to start being more conservative in any changes 
targeted at JDK 8.  There is a
small window of opportunity remaining for a performance-refactoring of 
VisibleMemberMap, but to be conservative, we cannot consider a 
refactoring that also changes the generated output. If you can cite 
specific bugs in the currently generated output, then we can consider 
accepting appropriate bug fixes for those issues.

-- Jon

[1] http://mreinhold.org/blog/jdk8-preview