Improve performance of initial class scans (#1238)

* Improve performance of initial class scans * Limit scan to public classes Co-authored-by: wcekan <wcekan@verizonmedia.com>
yahoo · Mar 26, 2020 · 7048968 · 7048968
1 parent 9b0b2fc
commit 7048968
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 40 deletions.
diff --git a/elide-core/src/main/java/com/yahoo/elide/Elide.java b/elide-core/src/main/java/com/yahoo/elide/Elide.java
@@ -97,7 +97,7 @@ public Elide(ElideSettings elideSettings) {
         registerCustomSerde();
     }
 
-    private void registerCustomSerde() {
+    protected void registerCustomSerde() {
         Set<Class<?>> classes = ClassScanner.getAnnotatedClasses(ElideTypeConverter.class);
 
         for (Class<?> clazz : classes) {
@@ -114,7 +114,7 @@ private void registerCustomSerde() {
                     | NoSuchMethodException | InvocationTargetException e) {
                 String errorMsg = String.format("Error while registering custom Serde: %s", e.getLocalizedMessage());
                 log.error(errorMsg);
-                throw new UnableToAddSerdeException(errorMsg);
+                throw new UnableToAddSerdeException(errorMsg, e);
             }
             ElideTypeConverter converter = clazz.getAnnotation(ElideTypeConverter.class);
             Class baseType = converter.type();
@@ -130,13 +130,13 @@ private void registerCustomSerde() {
         }
     }
 
-    private void registerCustomSerde(Class<?> type, Serde serde, String name) {
+    protected void registerCustomSerde(Class<?> type, Serde serde, String name) {
         log.info("Registering serde for type : {}", type);
         CoerceUtil.register(type, serde);
         registerCustomSerdeInObjectMapper(type, serde, name);
     }
 
-    private void registerCustomSerdeInObjectMapper(Class<?> type, Serde serde, String name) {
+    protected void registerCustomSerdeInObjectMapper(Class<?> type, Serde serde, String name) {
         ObjectMapper objectMapper = mapper.getObjectMapper();
         objectMapper.registerModule(new SimpleModule(name)
                 .addSerializer(type, new JsonSerializer<Object>() {

diff --git a/elide-core/src/main/java/com/yahoo/elide/core/exceptions/UnableToAddSerdeException.java b/elide-core/src/main/java/com/yahoo/elide/core/exceptions/UnableToAddSerdeException.java
@@ -9,4 +9,8 @@ public class UnableToAddSerdeException extends RuntimeException {
     public UnableToAddSerdeException(String message) {
         super(message);
     }
+
+    public UnableToAddSerdeException(String message, Throwable cause) {
+        super(message, cause);
+    }
 }
diff --git a/elide-core/src/main/java/com/yahoo/elide/utils/ClassScanner.java b/elide-core/src/main/java/com/yahoo/elide/utils/ClassScanner.java
@@ -20,7 +20,7 @@ public class ClassScanner {
     /**
      * Scans all classes accessible from the context class loader which belong to the given package and subpackages.
      *
-     * @param toScan     package to scan
+     * @param toScan package to scan
      * @param annotation Annotation to search
      * @return The classes
      */
@@ -32,11 +32,12 @@ static public Set<Class<?>> getAnnotatedClasses(Package toScan, Class<? extends
      * Scans all classes accessible from the context class loader which belong to the given package and subpackages.
      *
      * @param packageName package name to scan.
-     * @param annotation  Annotation to search
+     * @param annotation Annotation to search
      * @return The classes
      */
     static public Set<Class<?>> getAnnotatedClasses(String packageName, Class<? extends Annotation> annotation) {
-        try (ScanResult scanResult = new ClassGraph().enableAllInfo().whitelistPackages(packageName).scan()) {
+        try (ScanResult scanResult = new ClassGraph()
+                .enableClassInfo().enableAnnotationInfo().whitelistPackages(packageName).scan()) {
             return scanResult.getClassesWithAnnotation(annotation.getCanonicalName()).stream()
                     .map((ClassInfo::loadClass))
                     .collect(Collectors.toSet());
@@ -46,11 +47,12 @@ static public Set<Class<?>> getAnnotatedClasses(String packageName, Class<? exte
     /**
      * Scans all classes accessible from the context class loader which belong to the current class loader.
      *
-     * @param annotation  Annotation to search
+     * @param annotation Annotation to search
      * @return The classes
      */
     static public Set<Class<?>> getAnnotatedClasses(Class<? extends Annotation> annotation) {
-        try (ScanResult scanResult = new ClassGraph().enableAllInfo().scan()) {
+        try (ScanResult scanResult = new ClassGraph()
+                .enableClassInfo().enableAnnotationInfo().scan()) {
             return scanResult.getClassesWithAnnotation(annotation.getCanonicalName()).stream()
                     .map((ClassInfo::loadClass))
                     .collect(Collectors.toSet());
@@ -63,7 +65,8 @@ static public Set<Class<?>> getAnnotatedClasses(Class<? extends Annotation> anno
      * @return All the classes within a package.
      */
     static public Set<Class<?>> getAllClasses(String packageName) {
-        try (ScanResult scanResult = new ClassGraph().enableAllInfo().whitelistPackages(packageName).scan()) {
+        try (ScanResult scanResult = new ClassGraph()
+                .enableClassInfo().whitelistPackages(packageName).scan()) {
             return scanResult.getAllClasses().stream()
                     .map((ClassInfo::loadClass))
                     .collect(Collectors.toSet());

diff --git a/elide-core/src/test/java/com/yahoo/elide/ElideCustomSerdeRegistrationTest.java b/elide-core/src/test/java/com/yahoo/elide/ElideCustomSerdeRegistrationTest.java
@@ -24,21 +24,21 @@ class DummyTwo extends Dummy {
 class DummyThree extends Dummy {
 }
 
-@ElideTypeConverter(type = Dummy.class, name = "Dummy", subTypes = {DummyThree.class, DummyTwo.class})
-class DummySerde implements Serde<String, Dummy> {
-
-    @Override
-    public Dummy deserialize(String val) {
-        return null;
-    }
-
-    @Override
-    public String serialize(Dummy val) {
-        return null;
+public class ElideCustomSerdeRegistrationTest {
+    @ElideTypeConverter(type = Dummy.class, name = "Dummy", subTypes = { DummyThree.class, DummyTwo.class })
+    public static class DummySerde implements Serde<String, Dummy> {
+
+        @Override
+        public Dummy deserialize(String val) {
+            return null;
+        }
+
+        @Override
+        public String serialize(Dummy val) {
+            return null;
+        }
     }
-}
 
-public class ElideCustomSerdeRegistrationTest {
     @Test
     public void testRegisterCustomSerde() {
         HashMapDataStore wrapped = new HashMapDataStore(Dummy.class.getPackage());

diff --git a/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java b/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java
@@ -105,17 +105,17 @@ public void testFindCheckByExpression() {
         assertEquals("Prefab.Common.UpdateOnCreate", getCheckIdentifier(UpdateOnCreate.class));
     }
 
-    @Test
-    public void testCheckScan() {
-
-        @SecurityCheck("User is Admin")
-        class Foo extends UserCheck {
+    @SecurityCheck("User is Admin")
+    public class Foo extends UserCheck {
 
-            @Override
-            public boolean ok(com.yahoo.elide.security.User user) {
-                return false;
-            }
+        @Override
+        public boolean ok(com.yahoo.elide.security.User user) {
+            return false;
         }
+    }
+
+    @Test
+    public void testCheckScan() {
 
         EntityDictionary testDictionary = new EntityDictionary(new HashMap<>());
         testDictionary.scanForSecurityChecks();

diff --git a/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java b/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java
@@ -27,18 +27,14 @@ public void testGetAllClasses() {
     @Test
     public void testGetAnnotatedClasses() {
         Set<Class<?>> classes = ClassScanner.getAnnotatedClasses("example", ReadPermission.class);
-        assertEquals(6, classes.size());
-        for (Class<?> cls : classes) {
-            assertTrue(cls.isAnnotationPresent(ReadPermission.class));
-        }
+        assertEquals(6, classes.size(), "Actual: " + classes);
+        classes.forEach(cls -> assertTrue(cls.isAnnotationPresent(ReadPermission.class)));
     }
 
     @Test
     public void testGetAllAnnotatedClasses() {
         Set<Class<?>> classes = ClassScanner.getAnnotatedClasses(ReadPermission.class);
-        assertEquals(20, classes.size());
-        for (Class<?> cls : classes) {
-            assertTrue(cls.isAnnotationPresent(ReadPermission.class));
-        }
+        assertEquals(12, classes.size(), "Actual: " + classes);
+        classes.forEach(cls -> assertTrue(cls.isAnnotationPresent(ReadPermission.class)));
     }
 }