Preliminary RFR (L): 8001107: @Stable annotation for constant folding of lazily evaluated variables

[Rickard Bäckman]

Vladimir,

first, nice work.

A couple of comments about the code:

ciArray.hpp / ciArray.cpp
Looks like most of the new methods could be declared const.

ciConstant.hpp
I would like to have braces add to the if / else if / else

classFileParser.hpp
The is_stable() should be declared const.

memnode.cpp
You are adding plenty of logic to an already very long method, could we extract the new parts to its own method? or methods.
The elembt local variable is never really used.

type.cpp
2 instances of assert(stable == true || stable == false, ""); - this seems to be always true?
The constant 42 in the hash, since we are hashing pointers it might be more effective to use and odd number?

3505: if (stable_dimension <= 0 || stable_dimension == 1 && stable == this->stable())
should have parantheses added to clarify the || && logic.

3511   if (stable_dimension > 1 && elem_ptr != NULL && elem_ptr->base() == Type::AryPtr) {
could use the is_aryptr() instead of comparing base to Type::AryPtr 

type.hpp
the getter is named stable(), in all other places it is is_stable() maybe we could rename stable -> is_stable.

Cheers
/R

On Jul 20, 2013, at 12:19 AM, Vladimir Ivanov wrote:

> http://cr.openjdk.java.net/~vlivanov/8001107/
> 1455 lines changed: 1366 ins; 34 del; 55 mod;
> 
> 8001107: @Stable annotation for constant folding of lazily evaluated variables
> 
> Excerpt from Javadoc:
> [@Stable annotation is] Internal marker for some fields in the JSR 292 implementation. A field may be annotated as stable if all of its component variables changes value at most once. A field's value counts as its component value. If the field is typed as an array, then all the non-null components of the array, of depth up to the rank of the field's array type, also count as component values. By extension, any variable (either array or field) which has annotated as stable is called a stable variable, and its non-null or non-zero
> value is called a stable value.
> 
> Testing: unit test, JPRT, VM testbase, nashorn, octane
> 
> Contributed-by: jrose, vlivanov
> 
> I marked review request as preliminary, because I think it may be too late to get it into 8. It'll be targeted for 8update then.
> 
> Thanks!
> 
> Best regards,
> Vladimir Ivanov