From 485dce987c6ab83464d456f51bb83e6c9dbcc8a1 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 24 Mar 2015 16:40:42 +0300 Subject: [PATCH] Fix byte array memory leak from MemoryBasedClassLoader 40 Mb of bytes of preloaded compiler classes during the compilation become 16 Mb after this change This is a slightly reworked version of 58b033d (reverted in 3f05419). The problem in that commit was that URLClassLoader did not work in the fashion MemoryBasedClassLoader was designed to work (child first, parent last). So some resource was found in an incorrect jar which was causing the compiler to break --- .../preloading/ClassPreloadingUtils.java | 14 ++++++++--- .../preloading/MemoryBasedClassLoader.java | 24 ++++++++++++++++++- .../org/jetbrains/kotlin/utils/PathUtil.java | 6 ++++- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/compiler/preloader/src/org/jetbrains/kotlin/preloading/ClassPreloadingUtils.java b/compiler/preloader/src/org/jetbrains/kotlin/preloading/ClassPreloadingUtils.java index b4b866d94da..dfbc800316c 100644 --- a/compiler/preloader/src/org/jetbrains/kotlin/preloading/ClassPreloadingUtils.java +++ b/compiler/preloader/src/org/jetbrains/kotlin/preloading/ClassPreloadingUtils.java @@ -17,6 +17,8 @@ package org.jetbrains.kotlin.preloading; import java.io.*; +import java.net.URL; +import java.net.URLClassLoader; import java.util.*; import java.util.jar.Attributes; import java.util.jar.JarFile; @@ -29,8 +31,6 @@ public class ClassPreloadingUtils { /** * Creates a class loader that loads all classes from {@code jarFiles} into memory to make loading faster (avoid skipping through zip archives). * - * NOTE: if many resources with the same name exist, only the first one will be loaded - * * @param jarFiles jars to load all classes from * @param classCountEstimation an estimated number of classes in a the jars * @param parentClassLoader parent class loader @@ -53,7 +53,15 @@ public class ClassPreloadingUtils { parentClassLoader = preloadClasses(classpath, classCountEstimation, parentClassLoader, null, handler); } - return new MemoryBasedClassLoader(classesToLoadByParent, parentClassLoader, entries, handler); + return new MemoryBasedClassLoader(classesToLoadByParent, parentClassLoader, entries, handler, createFallbackClassLoader(jarFiles)); + } + + private static URLClassLoader createFallbackClassLoader(Collection files) throws IOException { + List urls = new ArrayList(files.size()); + for (File file : files) { + urls.add(file.toURI().toURL()); + } + return new URLClassLoader(urls.toArray(new URL[urls.size()]), null); } public static ClassLoader preloadClasses( diff --git a/compiler/preloader/src/org/jetbrains/kotlin/preloading/MemoryBasedClassLoader.java b/compiler/preloader/src/org/jetbrains/kotlin/preloading/MemoryBasedClassLoader.java index 3c30af56cbd..8b6efa9334c 100644 --- a/compiler/preloader/src/org/jetbrains/kotlin/preloading/MemoryBasedClassLoader.java +++ b/compiler/preloader/src/org/jetbrains/kotlin/preloading/MemoryBasedClassLoader.java @@ -21,23 +21,34 @@ import java.net.URL; import java.util.*; @SuppressWarnings("unchecked") +/** + * A class loader which loads classes and resources from the given map. + * + * To save memory, as soon as any class is loaded, its bytecode is removed from the map. + * This means that once any class is loaded, it _cannot be found_ as a resource anymore. + * Therefore if you need to be able to find classes via findResource(), you should pass a fallback + * class loader which is able to do that at any point of time. + */ public class MemoryBasedClassLoader extends ClassLoader { private final ClassCondition classesToLoadByParent; private final ClassLoader parent; private final Map preloadedResources; private final ClassHandler handler; + private final ClassLoader fallbackResourceLoader; public MemoryBasedClassLoader( ClassCondition classesToLoadByParent, ClassLoader parent, Map preloadedResources, - ClassHandler handler + ClassHandler handler, + ClassLoader fallbackResourceLoader ) { super(null); this.classesToLoadByParent = classesToLoadByParent; this.parent = parent; this.preloadedResources = preloadedResources; this.handler = handler; + this.fallbackResourceLoader = fallbackResourceLoader; } @Override @@ -77,6 +88,9 @@ public class MemoryBasedClassLoader extends ClassLoader { Object resources = preloadedResources.get(internalName); if (resources == null) return null; + // Clear the resource, we won't need it anymore + preloadedResources.remove(internalName); + ResourceData resourceData = resources instanceof ResourceData ? ((ResourceData) resources) : ((List) resources).get(0); @@ -98,6 +112,10 @@ public class MemoryBasedClassLoader extends ClassLoader { @Override public URL getResource(String name) { URL resource = super.getResource(name); + if (resource == null) { + resource = fallbackResourceLoader.getResource(name); + } + if (resource == null && parent != null) { return parent.getResource(name); } @@ -113,6 +131,10 @@ public class MemoryBasedClassLoader extends ClassLoader { @Override public Enumeration getResources(String name) throws IOException { Enumeration own = super.getResources(name); + if (!own.hasMoreElements()) { + own = fallbackResourceLoader.getResources(name); + } + if (parent == null) return own; Enumeration fromParent = parent.getResources(name); diff --git a/compiler/util/src/org/jetbrains/kotlin/utils/PathUtil.java b/compiler/util/src/org/jetbrains/kotlin/utils/PathUtil.java index 2c18b341340..a8af90c9148 100644 --- a/compiler/util/src/org/jetbrains/kotlin/utils/PathUtil.java +++ b/compiler/util/src/org/jetbrains/kotlin/utils/PathUtil.java @@ -135,7 +135,11 @@ public class PathUtil { @NotNull public static File getResourcePathForClass(@NotNull Class aClass) { - String resourceRoot = PathManager.getResourceRoot(aClass, "/" + aClass.getName().replace('.', '/') + ".class"); + String path = "/" + aClass.getName().replace('.', '/') + ".class"; + String resourceRoot = PathManager.getResourceRoot(aClass, path); + if (resourceRoot == null) { + throw new IllegalStateException("Resource not found: " + path); + } return new File(resourceRoot).getAbsoluteFile(); }