Review request for JDK-8168373: don't emit conversions for symbols outside their lexical scope

Sundararajan Athijegannathan sundararajan.athijegannathan at oracle.com
Fri Nov 11 11:43:41 UTC 2016


+1


On 11/10/2016 5:50 PM, Attila Szegedi wrote:
> 2nd review, anyone? Or can I go ahead with one?
>
> Attila.
>
>> On 08 Nov 2016, at 18:07, Hannes Wallnöfer <hannes.wallnoefer at oracle.com> wrote:
>>
>> +1
>>
>> Thank for fixing this, Attila!
>>
>> Hannes
>>
>>> Am 08.11.2016 um 14:01 schrieb Attila Szegedi <szegedia at gmail.com>:
>>>
>>> Please review JDK-8168373 "don't emit conversions for symbols outside their lexical scope" at <http://cr.openjdk.java.net/~attila/8168373/webrev.jdk9> for <https://bugs.openjdk.java.net/browse/JDK-8168373>
>>>
>>> Few remarks on the implementation:
>>> - I changed localVariableTypes from using IdentityHashMap to HashMap. The reason for this is that Symbol uses identity semantics for equals/hashCode anyway, and IdentityHashMap.keySet() has a worse implementation of removeAll, which is now used. HashMap has O(min(m, n)) where “m” and “n” are sizes of the two collections involved in removeAll, while IdentityHashMap.keySet() is always O(n) to the size of the map.
>>>
>>> - in the “-462,13” patch block: we now clone localVariableTypes at most once, instead of once per symbol that needs to be set in it (that was happening in the setType() call). This is an optimization fix.
>>>
>>> - in the “-1044,19” patch block there’s no longer a need to manually remove exceptionSymbol from the localVariableTypes when following the CF edge from a catch body to the try statement's end label. It becomes part of the general case handling lexically scoped symbols; leaveBlock on catchBody will take care of it.
>>>
>>> - the PrintVisitor change now correctly prints “const” and “let” nodes. I was using it for debugging and it was confusing that it printed “var” for “let”.
>>>
>>> Thanks,
>>> Attila.
>>>



More information about the nashorn-dev mailing list