RFR (XL): 8139170: JVMCI refresh
Christian Thalinger
christian.thalinger at oracle.com
Tue Oct 13 17:32:49 UTC 2015
> On Oct 12, 2015, at 6:19 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
> Since we will get more changes from labs later we may enumerate them: JVMCI refresh 1.
This will be the only universal refresh. If there is something later that needs to be fixed we will have separate bugs or enhancements.
>
> Can you explain new sed command in Gensrc-jdk.vm.ci.gmk. Are you sure it works on windows?
>
> Instead of is_trivial(method) may be we should have general function called from AdvancedThresholdPolicy::common() which return compilation level for particular method (for example, we can also limit it using compilecommand). Could be done as separate change after this.
>
> I looked on jdk.vm.ci changes and nothing looks terrible wrong. I am not familiar with it so I can't say that each line is correct. One thing I am wondering is why explicit imports are used instead of .* (compilation speed?):
>
> -import java.lang.annotation.*;
> +import java.lang.annotation.ElementType;
> +import java.lang.annotation.Retention;
> +import java.lang.annotation.RetentionPolicy;
> +import java.lang.annotation.Target;
>
> An other thing is using AMD64/amd64 in files and directory names. May be we should rename it to x64.
No. The official name of this architecture is x86_64. Java’s os.arch returns amd64 and (unfortunately) x86_64 on Mac OS X.
>
> jvmciCodeInstaller_x86.cpp and jvmciCodeInstaller.cpp has next pattern:
> + if (HotSpotMetaspaceConstantImpl::compressed(constant)) {
> +#ifdef _LP64
> + ...
> +#else
> + fatal("unexpected compressed Klass* in 32-bit mode");
> +#endif
> + } else {
>
> It would be nice to hide fatal() in some wrapper function.
>
> jvmciEnv.cpp use ASSERT instead of DEBUG. We renamed ifdef DEBUG long ago.
Yes, I noticed that too. Will change to ASSERT.
>
> globals.hpp changes conflict with 8139377.
>
> Tests changes (mostly renaming) looks fine.
>
> Fix copyright years (2015, 2015) in new files.
Will fix that.
>
> Thanks,
> Vladimir
>
> On 10/13/15 10:19 AM, Christian Thalinger wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8139170
>> http://cr.openjdk.java.net/~twisti/8139170/webrev/
>>
>> During the review period for JEP 243 there were some changes and enhancements to the JVMCI code done by Oracle Labs. In order to not disturb the already long and complicated review of JEP 243 we decided to do a refresh after the initial integration.
>>
>> A lot of the Java changes is switching to using explicit imports.
>>
More information about the hotspot-compiler-dev
mailing list