> On 1 Dec 2017, at 12:16, mandy chung <mandy.ch...@oracle.com> wrote: > > > > On 11/30/17 12:17 PM, Paul Sandoz wrote: >> Here is the updated webrev: >> >> >> http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-get/webrev/ >> >> >> I opted for the simple solution using a LinkedHashSet. >> > > This fix looks good except a typo in the test: > > 94 "Class %s does not have extracly one field: %d", c.getName(), nfs)); > > s/extracly/exactly/ >
Fixed. > I am wondering if these @run should run with both othervm and agentvm mode > since > it currently depends on the jtreg command-line > (I believe our test target uses > agentvm as the default). > It does. * @run main/othervm StaticFieldsOnInterface C * @run main/othervm StaticFieldsOnInterface D * @run main/othervm StaticFieldsOnInterface Y This ok? >> It’s possible to heavily optimize (avoiding the production of a linked hash >> set until required [*]) but the resulting code is harder to understand. >> > > I was tempted to come up with optimization too when first reading the patch > but I am with you. I like the resulting code simple and clear. The > difference is an array list vs linked hash set which we should wait until > this is really a performance issue. FWIW getMethods implementation also > creates a linked hash set if not cached. > Ok, thanks, Paul.