8048079: Persistent code store is broken after optimistic types merge
authorhannesw
Wed, 25 Jun 2014 14:36:24 +0200
changeset 25243 7a1edca6ce94
parent 25242 ac1d21c4d61d
child 25244 627d7e86f3b5
8048079: Persistent code store is broken after optimistic types merge Reviewed-by: sundar, jlaskey, attila
nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java
nashorn/src/jdk/nashorn/internal/runtime/Context.java
nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java
nashorn/test/script/basic/JDK-8048079_1.js
nashorn/test/script/basic/JDK-8048079_1.js.EXPECTED
nashorn/test/script/basic/JDK-8048079_2.js
nashorn/test/script/basic/JDK-8048079_2.js.EXPECTED
nashorn/test/src/jdk/nashorn/internal/runtime/CodeStoreAndPathTest.java
--- a/nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java	Wed Jun 25 17:08:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java	Wed Jun 25 14:36:24 2014 +0200
@@ -502,7 +502,7 @@
             Class<?> rootClass = null;
             long length = 0L;
 
-            final CodeInstaller<?> codeInstaller = compiler.getCodeInstaller();
+            final CodeInstaller<ScriptEnvironment> codeInstaller = compiler.getCodeInstaller();
 
             final Map<String, byte[]> bytecode = compiler.getBytecode();
 
@@ -527,12 +527,10 @@
             final Object[] constants = compiler.getConstantData().toArray();
             codeInstaller.initialize(installedClasses.values(), compiler.getSource(), constants);
 
-            // index recompilable script function datas in the constant pool
-            final Map<RecompilableScriptFunctionData, RecompilableScriptFunctionData> rfns = new IdentityHashMap<>();
+            // initialize transient fields on recompilable script function data
             for (final Object constant: constants) {
                 if (constant instanceof RecompilableScriptFunctionData) {
-                    final RecompilableScriptFunctionData rfn = (RecompilableScriptFunctionData)constant;
-                    rfns.put(rfn, rfn);
+                    ((RecompilableScriptFunctionData)constant).initTransients(compiler.getSource(), codeInstaller);
                 }
             }
 
--- a/nashorn/src/jdk/nashorn/internal/runtime/Context.java	Wed Jun 25 17:08:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/Context.java	Wed Jun 25 14:36:24 2014 +0200
@@ -444,15 +444,11 @@
         }
 
         if (env._persistent_cache) {
-            if (env._lazy_compilation || env._optimistic_types) {
-                getErr().println("Can not use persistent class caching with lazy compilation or optimistic compilation.");
-            } else {
-                try {
-                    final String cacheDir = Options.getStringProperty("nashorn.persistent.code.cache", "nashorn_code_cache");
-                    codeStore = new CodeStore(cacheDir);
-                } catch (final IOException e) {
-                    throw new RuntimeException("Error initializing code cache", e);
-                }
+            try {
+                final String cacheDir = Options.getStringProperty("nashorn.persistent.code.cache", "nashorn_code_cache");
+                codeStore = new CodeStore(cacheDir);
+            } catch (final IOException e) {
+                throw new RuntimeException("Error initializing code cache", e);
             }
         }
 
@@ -1179,7 +1175,7 @@
 
         for (final Object constant : constants) {
             if (constant instanceof RecompilableScriptFunctionData) {
-                ((RecompilableScriptFunctionData) constant).setCodeAndSource(installedClasses, source);
+                ((RecompilableScriptFunctionData) constant).initTransients(source, installer);
             }
         }
 
--- a/nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Wed Jun 25 17:08:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Wed Jun 25 14:36:24 2014 +0200
@@ -28,7 +28,6 @@
 import static jdk.nashorn.internal.lookup.Lookup.MH;
 
 import java.io.IOException;
-import java.io.Serializable;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
@@ -56,7 +55,6 @@
 import jdk.nashorn.internal.runtime.logging.DebugLogger;
 import jdk.nashorn.internal.runtime.logging.Loggable;
 import jdk.nashorn.internal.runtime.logging.Logger;
-import jdk.nashorn.internal.scripts.JS;
 
 /**
  * This is a subclass that represents a script function that may be regenerated,
@@ -85,9 +83,6 @@
     /** Source from which FunctionNode was parsed. */
     private transient Source source;
 
-    /** Allows us to retrieve the method handle for this function once the code is compiled */
-    private MethodLocator methodLocator;
-
     /** Token of this function within the source. */
     private final long token;
 
@@ -239,15 +234,18 @@
     }
 
     /**
-     * Setter for code and source
+     * Initialize transient fields on deserialized instances
      *
-     * @param code   map of code, class name to class
      * @param source source
+     * @param installer code installer
      */
-    public void setCodeAndSource(final Map<String, Class<?>> code, final Source source) {
-        this.source = source;
-        if (methodLocator != null) {
-            methodLocator.setClass(code.get(methodLocator.getClassName()));
+    public void initTransients(final Source source, final CodeInstaller<ScriptEnvironment> installer) {
+        if (this.source == null && this.installer == null) {
+            this.source = source;
+            this.installer = installer;
+        } else if (this.source != source || this.installer != installer) {
+            // Existing values must be same as those passed as parameters
+            throw new IllegalArgumentException();
         }
     }
 
@@ -529,7 +527,6 @@
             throw new IllegalStateException(functionNode.getName() + " id=" + functionNode.getId());
         }
         addCode(functionNode);
-        methodLocator = new MethodLocator(functionNode);
     }
 
     private CompiledFunction addCode(final MethodHandle target, final Map<Integer, Type> invalidatedProgramPoints, final int fnFlags) {
@@ -592,12 +589,7 @@
         synchronized (code) {
             CompiledFunction existingBest = super.getBest(callSiteType, runtimeScope);
             if (existingBest == null) {
-                if(code.isEmpty() && methodLocator != null) {
-                    // This is a deserialized object, reconnect from method handle
-                    existingBest = addCode(methodLocator.getMethodHandle(), null, methodLocator.getFunctionFlags());
-                } else {
-                    existingBest = addCode(compileTypeSpecialization(callSiteType, runtimeScope), callSiteType);
-                }
+                existingBest = addCode(compileTypeSpecialization(callSiteType, runtimeScope), callSiteType);
             }
 
             assert existingBest != null;
@@ -709,48 +701,6 @@
         return true;
     }
 
-    /**
-     * Helper class that allows us to retrieve the method handle for this function once it has been generated.
-     */
-    private static class MethodLocator implements Serializable {
-        private transient Class<?> clazz;
-        private final String className;
-        private final String methodName;
-        private final MethodType methodType;
-        private final int functionFlags;
-
-        private static final long serialVersionUID = -5420835725902966692L;
-
-        MethodLocator(final FunctionNode functionNode) {
-            this.className  = functionNode.getCompileUnit().getUnitClassName();
-            this.methodName = functionNode.getName();
-            this.methodType = new FunctionSignature(functionNode).getMethodType();
-            this.functionFlags = functionNode.getFlags();
-
-            assert className != null;
-            assert methodName != null;
-        }
-
-        void setClass(final Class<?> clazz) {
-            if (!JS.class.isAssignableFrom(clazz)) {
-                throw new IllegalArgumentException();
-            }
-            this.clazz = clazz;
-        }
-
-        String getClassName() {
-            return className;
-        }
-
-        MethodHandle getMethodHandle() {
-            return MH.findStatic(LOOKUP, clazz, methodName, methodType);
-        }
-
-        int getFunctionFlags() {
-            return functionFlags;
-        }
-    }
-
     private void readObject(final java.io.ObjectInputStream in) throws IOException, ClassNotFoundException {
         in.defaultReadObject();
         createLogger();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8048079_1.js	Wed Jun 25 14:36:24 2014 +0200
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8048079: Persistent code store is broken after optimistic types merge
+ *
+ * @test
+ * @run
+ * @option -pcc
+ * @option -Dnashorn.persistent.code.cache=build/nashorn_code_cache
+ * @fork
+ */
+
+load(__DIR__ + 'prototype.js');
+load(__DIR__ + 'yui.js');
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8048079_1.js.EXPECTED	Wed Jun 25 14:36:24 2014 +0200
@@ -0,0 +1,3 @@
+parsed and compiled ok prototype.js
+parsed and compiled ok yui-min.js
+parsed and compiled ok yui.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8048079_2.js	Wed Jun 25 14:36:24 2014 +0200
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8048079: Persistent code store is broken after optimistic types merge
+ *
+ * @test
+ * @run
+ * @option -pcc
+ * @option -Dnashorn.persistent.code.cache=build/nashorn_code_cache
+ * @fork
+ */
+
+load(__DIR__ + 'prototype.js');
+load(__DIR__ + 'yui.js');
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8048079_2.js.EXPECTED	Wed Jun 25 14:36:24 2014 +0200
@@ -0,0 +1,3 @@
+parsed and compiled ok prototype.js
+parsed and compiled ok yui-min.js
+parsed and compiled ok yui.js
--- a/nashorn/test/src/jdk/nashorn/internal/runtime/CodeStoreAndPathTest.java	Wed Jun 25 17:08:47 2014 +0530
+++ b/nashorn/test/src/jdk/nashorn/internal/runtime/CodeStoreAndPathTest.java	Wed Jun 25 14:36:24 2014 +0200
@@ -96,7 +96,7 @@
     final String codeCache = "build/nashorn_code_cache";
     final String oldUserDir = System.getProperty("user.dir");
 
-    private static final String[] ENGINE_OPTIONS = new String[]{"--persistent-code-cache", "--optimistic-types=false", "--lazy-compilation=false"};
+    private static final String[] ENGINE_OPTIONS = new String[]{"--persistent-code-cache"};
 
     public void checkCompiledScripts(final DirectoryStream<Path> stream, int numberOfScripts) throws IOException {
         for (final Path file : stream) {