+1
Very nice work!
Am 2015-08-31 um 12:09 schrieb Attila Szegedi:
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.
