Use isInstanceOf everywhere. (#518)

* Use isInstanceOf everywhere.

This changes the code everwhere to use .isInstanceof instead of instanceof, since
this is needed for user objects to work. However, the code is *incredibly* slow,
so much so that RandomTests takes 60 seconds on my computer. I added some caches,
but that didn't really work well enough either. So the next thought is to add this
to the jarInfo perhaps, but even still, once user classes are added,that will increase
startup time even more, so I think this has to be solved in a more general way. I think
the next step is to very closely examine the code path of isInstanceOf, and see what
steps are there today, and figure up if any can be removed, replaced with more efficient
mechanisms, or at least further cached.

This will serve as the basis of the continued work on objects, but until this is fixed,
it utterly kills the runtime performance, and simply cannot be merged in to master.

* Add some caches to speed things up in the course of operation

* Add minor optimization to CClassType

If both compared classes are native, and one extends the other, we can
reliably say it is an instanceof, so just use the native java instanceof
mechanism. This only helps a tiny bit, but it does help, so this will
stay, even though this is not the correct solution.

* Probable fix for slowness.

This commit caches the native class along with the FQCN, which can be used to
seriously speed up the lookup elsewhere, particularly when using instanceof.
The only exception to this is DynamicEnums, but there are so few of those that
it doesn't cause enough of a performance hit. This speeds up RandomTests by an
order of magnitude, but there were other performance increases that should have
helped some as well (though not an order of magnitude.) The end result is that
this should actually be a bit faster than the original code anyways.

* Add better logging when the NoClassDefError is thrown

src/main/java/com/laytonsmith/PureUtilities/ClassLoading/ClassDiscovery.java

@@ -11,6 +11,7 @@
 import com.laytonsmith.PureUtilities.Common.FileUtil;
 import com.laytonsmith.PureUtilities.Common.StreamUtils;
 import com.laytonsmith.PureUtilities.Common.StringUtils;
+import com.laytonsmith.PureUtilities.Pair;
 import com.laytonsmith.PureUtilities.ProgressIterator;
 import com.laytonsmith.PureUtilities.ZipIterator;
 import java.io.File;
@@ -139,6 +140,8 @@ public ClassDiscovery() {
 	 * Cache for constructor annotations. Whenever a new URL is added to the URL cache, this is cleared.
 	 */
 	private final Map<Class<? extends Annotation>, Set<ConstructorMirror<?>>> constructorAnnotationCache = new HashMap<>();
+	private final Map<Pair<Class<? extends Annotation>, Class<?>>, Set<ClassMirror<?>>>
+			classesWithAnnotationThatExtendCache = new HashMap<>();
 	/**
 	 * By default null, but this can be set per instance.
 	 */
@@ -462,6 +465,7 @@ public void invalidateCaches() {
 		fieldAnnotationCache.clear();
 		methodAnnotationCache.clear();
 		constructorAnnotationCache.clear();
+		classesWithAnnotationThatExtendCache.clear();
 		dirtyURLs.addAll(urlCache);
 	}
 
@@ -717,6 +721,15 @@ public Set<ClassMirror<?>> getClassesWithAnnotation(Class<? extends Annotation>
 	 */
 	@SuppressWarnings("unchecked")
 	public <T> Set<ClassMirror<? extends T>> getClassesWithAnnotationThatExtend(Class<? extends Annotation> annotation, Class<T> superClass) {
+		Pair<Class<? extends Annotation>, Class<?>> id = new Pair<>(annotation, superClass);
+		if(classesWithAnnotationThatExtendCache.containsKey(id)) {
+			// This (insane) double cast is necessary, because the cache will certainly contain the value of the
+			// correct type,
+			// but there's no way for us to encode T into the generic type of the definition, so we just do this,
+			// lie to the compiler, and go about our merry way. We do the same below.
+			// I'm totally open to a better approach though.
+			return (Set<ClassMirror<? extends T>>) (Object) classesWithAnnotationThatExtendCache.get(id);
+		}
 		Set<ClassMirror<? extends T>> mirrors = new HashSet<>();
 		for(ClassMirror<?> c : getClassesWithAnnotation(annotation)) {
 			if(doesClassExtend(c, superClass)) {
@@ -728,6 +741,7 @@ public <T> Set<ClassMirror<? extends T>> getClassesWithAnnotationThatExtend(Clas
 			// ourselves here.
 			mirrors.add(new ClassMirror<>(superClass));
 		}
+		classesWithAnnotationThatExtendCache.put(id, (Set<ClassMirror<?>>) (Object) mirrors);
 		return mirrors;
 	}
 

src/main/java/com/laytonsmith/PureUtilities/ClassLoading/ClassMirror/ClassMirror.java

@@ -457,7 +457,7 @@ public Class<T> loadClass() throws NoClassDefFoundError {
 		try {
 			return info.classReferenceMirror.loadClass();
 		} catch (ClassNotFoundException ex) {
-			throw new NoClassDefFoundError();
+			throw new NoClassDefFoundError(ex.getMessage());
 		}
 	}
 

src/main/java/com/laytonsmith/PureUtilities/Common/ClassUtils.java

@@ -1,8 +1,10 @@
 package com.laytonsmith.PureUtilities.Common;
 
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -43,6 +45,7 @@ public static Class forCanonicalName(String className, boolean initialize, Class
 		return forCanonicalName(className, true, initialize, classLoader);
 	}
 
+	private static final Map<String, Class> CANONICAL_CLASS_CACHE = new ConcurrentHashMap<>();
 	/**
 	 * Private version, which accepts the useInitializer parameter.
 	 *
@@ -53,7 +56,11 @@ public static Class forCanonicalName(String className, boolean initialize, Class
 	 * @return
 	 * @throws ClassNotFoundException
 	 */
-	private static Class forCanonicalName(String className, boolean useInitializer, boolean initialize, ClassLoader classLoader) throws ClassNotFoundException {
+	private static Class forCanonicalName(String className, boolean useInitializer, boolean initialize,
+			ClassLoader classLoader) throws ClassNotFoundException {
+		if(CANONICAL_CLASS_CACHE.containsKey(className)) {
+			return CANONICAL_CLASS_CACHE.get(className);
+		}
 		if("void".equals(className)) {
 			return void.class;
 		}
@@ -144,6 +151,7 @@ private static Class forCanonicalName(String className, boolean useInitializer,
 				throw ex;
 			}
 		}
+		CANONICAL_CLASS_CACHE.put(className, c);
 		return c;
 	}
 

src/main/java/com/laytonsmith/abstraction/bukkit/BukkitMCCommand.java

@@ -253,7 +253,7 @@ public boolean handleCustomCommand(MCCommandSender sender, String label, String[
 				Mixed fret = closure.executeCallable(null, t, new CString(label, t), new CString(sender.getName(), t), cargs,
 						new CArray(t) // reserved for an obgen style command array
 				);
-				if(fret instanceof CBoolean) {
+				if(fret.isInstanceOf(CBoolean.class)) {
 					return ((CBoolean) fret).getBoolean();
 				}
 			} catch (ConfigRuntimeException cre) {

src/main/java/com/laytonsmith/core/ArgumentValidation.java

@@ -233,10 +233,10 @@ public static long getInt(Mixed c, Target t) {
 		if(c == null || c instanceof CNull) {
 			return 0;
 		}
-		if(c instanceof CInt) {
-			i = ((CInt) c).getInt();
-		} else if(c instanceof CBoolean) {
-			if(((CBoolean) c).getBoolean()) {
+		if(c.isInstanceOf(CInt.class)) {
+			i = getObject(c, t, CInt.class).getInt();
+		} else if(c.isInstanceOf(CBoolean.class)) {
+			if(getObject(c, t, CBoolean.class).getBoolean()) {
 				i = 1;
 			} else {
 				i = 0;

src/main/java/com/laytonsmith/core/FullyQualifiedClassName.java

@@ -6,11 +6,14 @@
 package com.laytonsmith.core;
 
 import com.laytonsmith.PureUtilities.Common.StringUtils;
+import com.laytonsmith.annotations.MEnum;
+import com.laytonsmith.annotations.typeof;
 import com.laytonsmith.core.compiler.CompilerEnvironment;
 import com.laytonsmith.core.constructs.NativeTypeList;
 import com.laytonsmith.core.constructs.Target;
 import com.laytonsmith.core.environments.Environment;
 import com.laytonsmith.core.exceptions.CRE.CRECastException;
+import com.laytonsmith.core.natives.interfaces.Mixed;
 import com.laytonsmith.core.objects.ObjectDefinitionNotFoundException;
 import com.laytonsmith.core.objects.ObjectDefinitionTable;
 import java.util.ArrayList;
@@ -26,6 +29,7 @@ public final class FullyQualifiedClassName implements Comparable<FullyQualifiedC
 	public static final String PATH_SEPARATOR = ".";
 
 	private final String fullyQualifiedName;
+	private Class<? extends Mixed> nativeClass;
 
 	private FullyQualifiedClassName(String name) {
 		Objects.requireNonNull(name, "The name passed in may not be null");
@@ -70,13 +74,56 @@ public static FullyQualifiedClassName forName(String unqualified, Target t, Envi
 	}
 
 	/**
-	 * If the class is known for sure to be within the default import list, this method can be used.
+	 * If the type represents an enum tagged with {@link MEnum}, then this method should be used, but is otherwise
+	 * identical to {@link #forNativeClass(java.lang.Class)}.
+	 * @param clazz
+	 * @return
+	 */
+	public static FullyQualifiedClassName forNativeEnum(Class<? extends Enum> clazz) {
+		MEnum m = clazz.getAnnotation(MEnum.class);
+		if(m == null) {
+			throw new Error("Native enum " + clazz + " does not provide an MEnum annotation");
+		}
+		String fqcn = m.value();
+		FullyQualifiedClassName f = new FullyQualifiedClassName(fqcn);
+		try {
+			f.nativeClass = NativeTypeList.getNativeEnumType(f).typeof().getNativeType();
+		} catch (ClassNotFoundException ex) {
+			// This can't happen, it would have already been the above error.
+			throw new Error(ex);
+		}
+		return f;
+	}
+
+	/**
+	 * If the type represents a native class, this method can be used. Not only does it never throw an exception
+	 * (except an Error, if the class does not define a typeof annotation), getNativeClass will return a reference
+	 * to the Class object, which is useful for shortcutting various operations.
+	 * @param clazz
+	 * @return
+	 */
+	public static FullyQualifiedClassName forNativeClass(Class<? extends Mixed> clazz) {
+		typeof t = clazz.getAnnotation(typeof.class);
+		if(t == null) {
+			throw new Error("Native class " + clazz + " does not provide a typeof annotation");
+		}
+		String fqcn = t.value();
+		FullyQualifiedClassName f = new FullyQualifiedClassName(fqcn);
+		f.nativeClass = clazz;
+		return f;
+	}
+
+	/**
+	 * If the class is known for sure to be within the default import list, this method can be used. If the native
+	 * class is available, this MUST not be used, as it causes too much of a performance hit. Instead, use
+	 * {@link #forNativeClass(java.lang.Class)} or {@link #forNativeEnum(java.lang.Class)}. DynamicEnums are not
+	 * specially supported, but they can safely use this method, though their use should be very limited.
 	 * @param unqualified The (potentially) unqualified type.
 	 * @param t The code target.
 	 * @return The FullyQualifiedClassName.
 	 * @throws CRECastException If the class type can't be found
 	 */
-	public static FullyQualifiedClassName forDefaultClasses(String unqualified, Target t) throws CRECastException {
+	private static FullyQualifiedClassName forDefaultClasses(String unqualified, Target t) throws CRECastException {
 		String fqcn = NativeTypeList.resolveNativeType(unqualified);
 		if(fqcn == null) {
 			throw new CRECastException("Cannot find \"" + unqualified + "\" type", t);
@@ -88,6 +135,8 @@ public static FullyQualifiedClassName forDefaultClasses(String unqualified, Targ
 	 * If you know for a fact that the name is already fully qualified, this step skips qualification. If you aren't
 	 * sure whether or not the name is fully qualified, don't use the method, the other methods will accept a fully
 	 * qualified class name, but not change it, but if it isn't fully qualified, then it will do so.
+	 *
+	 * If this represents a native class, use {@link #forNativeClass(java.lang.Class)} instead.
 	 * @param qualified
 	 * @return
 	 */
@@ -130,6 +179,14 @@ public boolean isTypeUnion() {
 		return this.fullyQualifiedName.contains("|");
 	}
 
+	/**
+	 * Returns the underlying native class, iff this is a native class.
+	 * @return
+	 */
+	public Class<? extends Mixed> getNativeClass() {
+		return nativeClass;
+	}
+
 	public String getSimpleName() {
 		List<String> parts = new ArrayList<>();
 		for(String t : fullyQualifiedName.split("\\|")) {

src/main/java/com/laytonsmith/core/MainSandbox.java

@@ -31,12 +31,14 @@ public static void main(String[] args) throws Exception {
 					|| m.getSimpleName().equals("CLabel")
 					|| m.getSimpleName().equals("CLabel")
 					|| m.getSimpleName().equals("AbstractCREException")
+					|| m.getSimpleName().equals("CSlice")
+					|| m.getSimpleName().equals("CArray")
 					) {
 				continue;
 			}
 			l.add(m.getSimpleName());
 		}
-		System.out.println(" instanceof (" + StringUtils.Join(l, "|") + ")([^a-zA-Z0-9])");
+		System.out.println(" instanceof (" + StringUtils.Join(l, "|") + ")(?![a-zA-Z0-9])");
 		System.out.println(".isInstanceOf($1.class)$2");
 	}
 

src/main/java/com/laytonsmith/core/Method.java

@@ -20,7 +20,7 @@
 public class Method extends Construct implements Callable {
 
 	@SuppressWarnings("FieldNameHidesFieldInSuperclass")
-	public static final CClassType TYPE = CClassType.get("ms.lang.Method");
+	public static final CClassType TYPE = CClassType.get(Method.class);
 
 	private final CClassType returnType;
 	private final String name;

src/main/java/com/laytonsmith/core/ObjectGenerator.java

@@ -286,7 +286,7 @@ public MCItemStack item(Mixed i, Target t, boolean legacy) {
 				}
 			} else {
 				Mixed type = item.get("type", t);
-				if(type instanceof CString) {
+				if(type.isInstanceOf(CString.class)) {
 					int seperatorIndex = type.val().indexOf(':');
 					if(seperatorIndex != -1) {
 						try {
@@ -634,7 +634,7 @@ public MCItemMeta itemMeta(Mixed c, MCMaterial mat, Target t) throws ConfigRunti
 					Mixed li = ma.get("lore", t);
 					if(li instanceof CNull) {
 						//do nothing
-					} else if(li instanceof CString) {
+					} else if(li.isInstanceOf(CString.class)) {
 						List<String> ll = new ArrayList<>();
 						ll.add(li.val());
 						meta.setLore(ll);
@@ -940,7 +940,7 @@ public MCItemMeta itemMeta(Mixed c, MCMaterial mat, Target t) throws ConfigRunti
 						Mixed color = ma.get("color", t);
 						if(color instanceof CArray) {
 							((MCPotionMeta) meta).setColor(color((CArray) color, t));
-						} else if(color instanceof CString) {
+						} else if(color.isInstanceOf(CString.class)) {
 							((MCPotionMeta) meta).setColor(StaticLayer.GetConvertor().GetColor(color.val(), t));
 						}
 					}
@@ -1193,7 +1193,7 @@ public Map<MCEnchantment, Integer> enchants(CArray enchantArray, Target t) {
 			Mixed value = enchantArray.get(key, t);
 			if(enchantArray.isAssociative()) {
 				etype = StaticLayer.GetEnchantmentByName(key);
-				if(etype != null && value instanceof CInt) {
+				if(etype != null && value.isInstanceOf(CInt.class)) {
 					ret.put(etype, Static.getInt32(value, t));
 					continue;
 				}
@@ -1304,7 +1304,7 @@ public MCPotionData potionData(CArray pd, Target t) {
 		boolean upgraded = false;
 		if(pd.containsKey("extended")) {
 			Mixed cext = pd.get("extended", t);
-			if(cext instanceof CBoolean) {
+			if(cext.isInstanceOf(CBoolean.class)) {
 				extended = ((CBoolean) cext).getBoolean();
 			} else {
 				throw new CREFormatException(
@@ -1313,7 +1313,7 @@ public MCPotionData potionData(CArray pd, Target t) {
 		}
 		if(pd.containsKey("upgraded")) {
 			Mixed cupg = pd.get("upgraded", t);
-			if(cupg instanceof CBoolean) {
+			if(cupg.isInstanceOf(CBoolean.class)) {
 				upgraded = ((CBoolean) cupg).getBoolean();
 			} else {
 				throw new CREFormatException(
@@ -1367,11 +1367,11 @@ public MCFireworkEffect fireworkEffect(CArray fe, Target t) {
 				} else {
 					for(Mixed color : ccolors.asList()) {
 						MCColor mccolor;
-						if(color instanceof CString) {
+						if(color.isInstanceOf(CString.class)) {
 							mccolor = StaticLayer.GetConvertor().GetColor(color.val(), t);
 						} else if(color instanceof CArray) {
 							mccolor = color((CArray) color, t);
-						} else if(color instanceof CInt && ccolors.size() == 3) {
+						} else if(color.isInstanceOf(CInt.class) && ccolors.size() == 3) {
 							// Appears to be a single color
 							builder.addColor(color(ccolors, t));
 							break;
@@ -1382,7 +1382,7 @@ public MCFireworkEffect fireworkEffect(CArray fe, Target t) {
 						builder.addColor(mccolor);
 					}
 				}
-			} else if(colors instanceof CString) {
+			} else if(colors.isInstanceOf(CString.class)) {
 				String split[] = colors.val().split("\\|");
 				if(split.length == 0) {
 					builder.addColor(MCColor.WHITE);
@@ -1406,9 +1406,9 @@ public MCFireworkEffect fireworkEffect(CArray fe, Target t) {
 					MCColor mccolor;
 					if(color instanceof CArray) {
 						mccolor = color((CArray) color, t);
-					} else if(color instanceof CString) {
+					} else if(color.isInstanceOf(CString.class)) {
 						mccolor = StaticLayer.GetConvertor().GetColor(color.val(), t);
-					} else if(color instanceof CInt && ccolors.size() == 3) {
+					} else if(color.isInstanceOf(CInt.class) && ccolors.size() == 3) {
 						// Appears to be a single color
 						builder.addFadeColor(color(ccolors, t));
 						break;
@@ -1418,7 +1418,7 @@ public MCFireworkEffect fireworkEffect(CArray fe, Target t) {
 					}
 					builder.addFadeColor(mccolor);
 				}
-			} else if(colors instanceof CString) {
+			} else if(colors.isInstanceOf(CString.class)) {
 				String split[] = colors.val().split("\\|");
 				for(String s : split) {
 					builder.addFadeColor(StaticLayer.GetConvertor().GetColor(s, t));
@@ -1504,7 +1504,7 @@ public MCRecipe recipe(Mixed c, Target t) {
 				}
 				int i = 0;
 				for(Mixed row : shaped.asList()) {
-					if(row instanceof CString && row.val().length() >= 1 && row.val().length() <= 3) {
+					if(row.isInstanceOf(CString.class) && row.val().length() >= 1 && row.val().length() <= 3) {
 						shape[i] = row.val();
 						i++;
 					} else {
@@ -1520,7 +1520,7 @@ public MCRecipe recipe(Mixed c, Target t) {
 				for(String key : shapedIngredients.stringKeySet()) {
 					MCMaterial mat = null;
 					Mixed ingredient = shapedIngredients.get(key, t);
-					if(ingredient instanceof CString) {
+					if(ingredient.isInstanceOf(CString.class)) {
 						mat = StaticLayer.GetMaterial(ingredient.val());
 						if(mat == null) {
 							// maybe legacy item format
@@ -1534,7 +1534,7 @@ public MCRecipe recipe(Mixed c, Target t) {
 								MSLog.GetLogger().w(MSLog.Tags.DEPRECATION, "Numeric item formats (eg. \"0:0\" are deprecated.", t);
 							} catch (NumberFormatException ex) {}
 						}
-					} else if(ingredient instanceof CInt) {
+					} else if(ingredient.isInstanceOf(CInt.class)) {
 						mat = StaticLayer.GetMaterialFromLegacy(Static.getInt32(ingredient, t), 0);
 						MSLog.GetLogger().w(MSLog.Tags.DEPRECATION, "Numeric item ingredients are deprecated.", t);
 					} else if(ingredient instanceof CArray) {
@@ -1553,7 +1553,7 @@ public MCRecipe recipe(Mixed c, Target t) {
 					throw new CREFormatException("Ingredients array is invalid.", t);
 				}
 				for(Mixed ingredient : ingredients.asList()) {
-					if(ingredient instanceof CString) {
+					if(ingredient.isInstanceOf(CString.class)) {
 						MCMaterial mat = StaticLayer.GetMaterial(ingredient.val());
 						if(mat == null) {
 							// maybe legacy item format
@@ -1581,7 +1581,7 @@ public MCRecipe recipe(Mixed c, Target t) {
 
 			case FURNACE:
 				Mixed input = recipe.get("input", t);
-				if(input instanceof CString) {
+				if(input.isInstanceOf(CString.class)) {
 					MCMaterial mat = StaticLayer.GetMaterial(input.val());
 					if(mat == null) {
 						throw new CREFormatException("Furnace input is invalid: " + input.val(), t);