Thanks Vladimir! /Claes On 2017-11-13 17:38, Vladimir Ivanov wrote:

Looks good!

Best regards, Vladimir Ivanov

On 11/13/17 7:34 PM, Claes Redestad wrote:

...

Hi,

  this patch factors out the BoundMethodHandle species data class generation to a new ClassSpecializer facility.

  While currently semantically neutral, this will make it possible to reuse the facility in other places.

  Webrev: http://cr.openjdk.java.net/~redestad/8184777/open.00/   Bug: https://bugs.openjdk.java.net/browse/JDK-8184777

  Performance wise this adds a very small (~20k bytecode) amount of work to the initialization costs of BMHs, which we expect will be more than repaid as we apply the ClassSpecializer elsewhere.

  Thanks!

/Claes

Hi Christoph, On 2017-11-13 18:56, Christoph Dreis wrote:

Hey Claes,

though far away from being an expert on the subject matter, I have some very minor comments if you don't mind.

ClassSpecializer.java L510: * For example, a concrete species for two reference and one integral bound values have a shape like the following: Should be imho: L510: * For example, a concrete species for two references and one integral bound value has a shape like the following:

LambdaFormBuffer.java: L333: if (oldFns.size() == 0) return this; Could be: L333: if (oldFns.isEmpty()) return this;

nice catches! Fixed and updated webrev in-place. http://cr.openjdk.java.net/~redestad/8184777/open.00/ Thanks! /Claes

Hi Christoph, On 2017-11-14 15:29, Christoph Dreis wrote:

I still think it should be called "one integral bound value" instead of "values" and "has" instead of "have" (at least in case a singular species is meant). You seem to have fixed the singular "reference" typo only in ClassSpecializer.java.

I read your comment too fast. Fixed!

While looking at ClassSpecializer again, I found two additional editorial things I wanted to raise:

1.) 364 * @return class name, which by default is {@code outer().topClass().getName() + "$Species_" + deriveTypeString(key)} 365 */ 366 protected String deriveClassName() { 367 return topClass.getName() + "$Species_" + deriveTypeString(); 368 }

The @return doesn't seem to match the implementation. This looks a bit weird at least.

Updated code to match documentation.

2.) 328 * and produces a value of the required type. Should be "produce a value" given that "supply" is used some lines above.

Fixed!

Again - I hope you don't mind these minor comments.

Don't mind at all. Some of the typos here were pre-existing, and while this is internal code, it surely doesn't hurt cleaning up comments. http://cr.openjdk.java.net/~redestad/8184777/open.01/ Thanks! /Claes