Review request for JDK-8139931: Introduce Operation objects in Dynalink instead of string encoding

[Hannes Wallnoefer]

Very nice that we can keep using switch statements!

+1

Hannes

Am 2015-10-21 um 12:48 schrieb Attila Szegedi:
> Here’s an updated webrev, please review: <http://cr.openjdk.java.net/~attila/8139931/webrev.jdk9.01>
>
> I removed CompositeOperation.contains and containsAny operations and in Nashorn replaced their use with a new NashornCallSiteDescriptor.getFirstStandardOperation method; this allowed us to go back to using switch statement everywhere in linker logic where we were able to use it earlier, resulting in smaller diff and overall nicer code.
>
> Also, following a discussion with Hannes, we now enforce in CompositeOperation constructor that no components can themselves be either a CompositeOperation or a NamedOperation, and we enforce in NamedOperation that its base operation can’t itself be a NamedOperation.
>
> Thanks,
>    Attila.
>
>> On Oct 20, 2015, at 12:29 PM, Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com> wrote:
>>
>> +1 with changes.
>>
>> On final modifier: nothing specific comes to mind. We can leave it non-final...
>>
>> -Sundar
>>
>> On 10/20/2015 3:09 PM, Attila Szegedi wrote:
>>>> On Oct 20, 2015, at 10:52 AM, Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com> wrote:
>>>>
>>>> * StandardOperation.java is missing copyright.
>>>>
>>>> * NamedOperation.java is missing copyright.
>>>>
>>>> * CompositeOperation.java is missing copyright.
>>> Indeed; added copyrights.
>>>
>>>> * Can CompositeOperation be final?
>>> Well, it’s hard to see why would someone want to subclass it, but I also don’t really see why someone shouldn’t be able to do it, to maybe add some language-specific information and still have it be recognized externally as CompositeOperation. (Same reasoning goes for NamedOperation, I guess). Do you have an rationale for making it final?
>>>
>>>> * Unrelated ArrayData change? Unused method?
>>> Yes. I was following a change in the parameter type from String to Operation and stumbled upon it.
>>>
>>>> * NashornCallSiteDescriptor may have explanation as to why 18 bits are sufficient for "program point" [now that flag bits are used for encoding operation enums as well]
>>> I agree, I added an explanation.
>>>
>>>> That's all I could find...
>>>>
>>>> +1
>>>>
>>>> -Sundar
>>>>
>>>> On 10/20/2015 1:41 PM, Attila Szegedi wrote:
>>>>> Please review JDK-8139931 "Introduce Operation objects in Dynalink instead of string encoding" at <http://cr.openjdk.java.net/~attila/8139931/webrev.jdk9> for <https://bugs.openjdk.java.net/browse/JDK-8139931>
>>>>>
>>>>> This is admittedly a big one. It is also the last in the pipeline of the internal Dynalink cleanups, so we’ve reached the end of that!
>>>>>
>>>>> Thanks,
>>>>>    Attila.