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

Attila Szegedi attila.szegedi at oracle.com
Wed Oct 21 10:48:52 UTC 2015


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



More information about the nashorn-dev mailing list