Review Request for MVT incubator module

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Fri Sep 15 14:01:59 UTC 2017



On 15/09/17 13:49, Remi Forax wrote:
> ----- Mail original -----
>> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
>> À: "mandy chung" <mandy.chung at oracle.com>, "valhalla-dev" <valhalla-dev at openjdk.java.net>
>> Envoyé: Vendredi 15 Septembre 2017 14:04:43
>> Objet: Re: Review Request for MVT incubator module
>> Hi Mandy, this looks like a solid piece of work, thanks!
>>
>> Few comments:
>>
> ...
>> * I also tried several test scenarios in which custom annotations called
>> ValueCapableClass were used instead of the 'official' one in the
>> incubator module, but I have not been able to come up with any real
>> issues - my concern was that the analysis that the VM applied to the
>> annotation is mostly textual, but there seem to be a couple of saving
>> factors: (i) if the EnableMVT flag is not used, the VM will not even
>> attempt to look at annotations, (ii) when the flag is used, the
>> incubator module (which is added automatically when the flag is set)
>> will automatically supplant any classes coming from the classpath - so
>> the right VCC will be used. In short, I have not been able to make the
>> VM 'trip up' on some maliciously written fake VCC anno. Which is good.
> maybe, create an annotation VCC in its own module, and use jlink to create your own jdk that included the module ?
That's what i did (more or less), but since MVT support add the 
incubator module by default, you end up with split package issues as 
you'd have jdk.incubator.mvt in two modules.
If EnableMVT is not passed, the VM will not even bother looking at the 
annotation...

Maurizio
>> Cheers
>> Maurizio
> regards,
> Rémi
>
>>
>> On 15/09/17 07:02, mandy chung wrote:
>>> Webrev:
>>>     http://cr.openjdk.java.net/~mchung/valhalla/webrevs/incubator/
>>>
>>> This patch proposes to define an incubator module [1] for MVT, named
>>> `jdk.incubator.mvt`.  Specifically this
>>> movesjvm.internal.value.ValueCapableClass and
>>> jdk.experimental.value.ValueType APIto a new jdk.incubator.mvt package.
>>>
>>> -XX:+EnableMVT continues to be the runtime option to enable MVT. I
>>> have changed the runtime to add jdk.incubator.mvt when MVT is
>>> enabled.  For compile-time, compiling @VCC classes would require to
>>> run javac with `--add-modules jdk.incubator.mvt` as incubator modules
>>> are not resolved by default.
>>>
>>> A couple points worth mentioning:
>>>
>>> java.base can't reference @VCC and ValueType class statically since
>>> they are now in a different module. In addition, @VCC can only be
>>> loaded after startup asVM startup can only load classes in java.base.
>>>
>>> jdk.incubator.mvtis defined by the platform loader (as we want it to
>>> be deprivileged for security reason) and so Class.forName to find
>>> classes in jdk.incubator.module should use the right loader.
>>>
>>> jdk.incubator.mvt.ValueType is basically a wrapper around the internal
>>> ValueTypeHolder class that is what java.base references in the
>>> implementation.  MethodHandleBuilder is kept as internal API for now.
>>> I leave it as a future exercise if we decide to expose that API in the
>>> incubator module.
>>>
>>> The tests are updated with @modules jdk.incubator.mvt or add access to
>>> jdk.experimental
>>>
>>> I temporarily downgrade the javac incubating module warning to a note
>>> so that -Werror has no effect. We will need to revisit this warning in
>>> JDK 10.
>>>
>>> thanks
>>> Mandy
>>> [1] http://openjdk.java.net/jeps/11



More information about the valhalla-dev mailing list