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

[Vladimir Ivanov]

Rickard,

Thank you for review.
See my answers inline.

Updated version:
http://cr.openjdk.java.net/~vlivanov/8001107/webrev.01

Best regards,
Vladimir Ivanov

On 7/23/13 2:46 PM, Rickard Bäckman wrote:
> 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.
ciObject::klass() isn't const, so newly introduced methods can't be made 
const as well.

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

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

> 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.
Done. Introduced fold_stable_ary_elem helper function.

> The elembt local variable is never really used.
Removed.

> type.cpp
> 2 instances of assert(stable == true || stable == false, ""); - this seems to be always true?
Agree. Removed.

> The constant 42 in the hash, since we are hashing pointers it might be more effective to use and odd number?
Done.

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

> 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
Done.

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

> 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
>