+1
> On 15 Sep 2015, at 17:29, Attila Szegedi <attila.szeg...@oracle.com> wrote:
>
> Folks, I’m preparing to backport this to 8u-dev. I had to prepare a separate
> webrev, as I had to change few things, namely:
>
> - I resolved a conflict in CompilationPhase.java stemming from Sundar’s
> 8055917 being committed to 8u-dev before this change (this was BTW also the
> reason poor Sundar himself had a conflict and had to manually prepare a
> 8u-dev webrev…)
> - Some omitted type parameters in 9 can’t be omitted in 8 because of weaker
> type parameter inference, hence some more explicit type specializations were
> used in few places
> - use of lambdas in RecompilableScriptFunctionData was not compatible with
> Nashorn using source -1.7 in 8u-dev.
>
> Please review so I can request a backport to 8u-dev.
>
> Thanks,
> Attila.
>
>> On Sep 1, 2015, at 9:14 AM, Marcus Lagergren <lagerg...@gmail.com> wrote:
>>
>> Reviewed new webrev. Happy +1
>>
>> /M
>>
>>> On 31 Aug 2015, at 12:09, Attila Szegedi <attila.szeg...@oracle.com> wrote:
>>>
>>> I posted another webrev, can you please review:
>>> http://cr.openjdk.java.net/~attila/8133300/webrev.01.jdk9
>>> <http://cr.openjdk.java.net/~attila/8133300/webrev.01.jdk9>
>>> The only changes are:
>>> - added JavaDoc comments explaining SerializedAst class, as per Marcus’
>>> suggestion
>>> - external symbols’ re-marking as global now only happens for non-cached
>>> split functions (previously, it happened for all non-cached functions).
>>> Other functions will already have external symbols marked as globals, as
>>> they’re coming from a lazy compilation.
>>>
>>> Thanks,
>>> Attila.
>>>
>>>> On Aug 31, 2015, at 10:58 AM, Attila Szegedi <attila.szeg...@oracle.com>
>>>> wrote:
>>>>
>>>> if the FunctionNode object is set as the cached representation of a
>>>> RecompilableScriptFunctionData using its setCached method, its IS_CACHED
>>>> flag is set to true. The flag is used in recompilation to determine the
>>>> necessary compilation phases: if the function is cached, it means phases
>>>> in Compiler.COMPILE_UPTO_CACHED were already run on it, so they don't need
>>>> to be repeated. Otherwise the function is freshly reparsed, so the full
>>>> compiler pipeline needs to be re-run.
>>>>
>>>> There’s also the issue of re-marking external symbols as globals in
>>>> RecompilableScriptFunctionData.cloneSymbols(). We normally don’t have to
>>>> do that – I implemented that back when I was caching pre-pass ASTs. Now
>>>> that we're only caching AST resulting from lazy compilation, we don't
>>>> really need it except for split functions, which are incidentally cached
>>>> from the eager pass :-)
>>>>
>>>> When we lazily compile a function from scratch, all symbols outside of it
>>>> are considered global (since AssignSymbols won't find them in the
>>>> enclosing lexical scope, as the enclosing source text is not reparsed).
>>>> OTOH, if we cache a FunctionNode resulting from the eager first pass, it
>>>> will have the full lexical scope, so those symbols won't be marked as
>>>> "global" (which is rather a misnomer at the moment and should be instead
>>>> considered “external” instead). We need the cached AST and a lazily parsed
>>>> AST to be identical though, otherwise things break down further down the
>>>> line.
>>>>
>>>> Split functions are cached from eager pre-pass (and before they were
>>>> serialized from eager pre-pass). They didn't suffer from this problem
>>>> though as AssignSymbols was re-run every time after deserialization. Now
>>>> however, I moved AssignSymbols to be a pre-cache phase.
>>>>
>>>> I'll modify the code though so that this marking of external symbols only
>>>> happens when we cache a split function.
>>>>
>>>> Attila.
>>>>
>>>>> On Aug 30, 2015, at 3:07 PM, Marcus Lagergren <mar...@lagergren.net>
>>>>> wrote:
>>>>>
>>>>> Can you elaborate a bit on the isCached mechanism in FunctionNode?
>>>>>
>>>>> /M
>>>>>
>>>>>> On 26 Aug 2015, at 14:18, Attila Szegedi <attila.szeg...@oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>> Please review JDK-8133300 "Ensure symbol table immutability in Nashorn
>>>>>> AST" at <http://cr.openjdk.java.net/~attila/8133300/webrev.jdk9> for
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8133300>
>>>>>>
>>>>>> Implementation notes for reviewers are here:
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8133300?focusedCommentId=13837384&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13837384
>>>>>>
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8133300?focusedCommentId=13837384&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13837384>>
>>>>>>
>>>>>> Thanks,
>>>>>> Attila.
>>>>>
>>>>
>>>
>>
>
