You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2019/10/01 12:51:10 UTC

[isis] branch v2 updated: ISIS-2158: spec-loading: simplify spec-cache

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch v2
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/v2 by this push:
     new b6fd5f7  ISIS-2158: spec-loading: simplify spec-cache
b6fd5f7 is described below

commit b6fd5f71c68a3f57eac0372ca6c69860dc4a814b
Author: Andi Huber <ah...@apache.org>
AuthorDate: Tue Oct 1 14:51:00 2019 +0200

    ISIS-2158: spec-loading: simplify spec-cache
---
 .../isis/config/registry/IsisBeanTypeRegistry.java |  4 ++-
 .../specloader/SpecificationCacheDefault.java      | 37 ---------------------
 .../specloader/SpecificationLoaderDefault.java     | 38 ++++------------------
 .../specloader/SpecificationCacheDefaultTest.java  |  6 ++--
 .../domainmodel/SpecloaderPerformanceTest.java     | 15 +++++++--
 5 files changed, 25 insertions(+), 75 deletions(-)

diff --git a/core/config/src/main/java/org/apache/isis/config/registry/IsisBeanTypeRegistry.java b/core/config/src/main/java/org/apache/isis/config/registry/IsisBeanTypeRegistry.java
index 2bf2674..3968931 100644
--- a/core/config/src/main/java/org/apache/isis/config/registry/IsisBeanTypeRegistry.java
+++ b/core/config/src/main/java/org/apache/isis/config/registry/IsisBeanTypeRegistry.java
@@ -131,6 +131,8 @@ public final class IsisBeanTypeRegistry implements BeanSortClassifier, AutoClose
         }
     }
 
+    public static boolean repeatedTesting = false;
+    
     /**
      * Implemented as a one-shot, that clears the inbox afterwards.
      * @return
@@ -141,7 +143,7 @@ public final class IsisBeanTypeRegistry implements BeanSortClassifier, AutoClose
 
         synchronized (inbox) {
             defensiveCopy = new HashMap<>(inbox);
-            inbox.clear();
+            if(!repeatedTesting) inbox.clear();
         }
 
         if(log.isDebugEnabled()) {
diff --git a/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefault.java b/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefault.java
index 647602a..bc574e3 100644
--- a/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefault.java
+++ b/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefault.java
@@ -24,36 +24,19 @@ import java.util.Collection;
 import java.util.Map;
 import java.util.function.Function;
 
-import org.apache.isis.commons.internal.collections._Lists;
 import org.apache.isis.commons.internal.collections._Maps;
 import org.apache.isis.metamodel.facets.object.objectspecid.ObjectSpecIdFacet;
 import org.apache.isis.metamodel.spec.ObjectSpecId;
 import org.apache.isis.metamodel.spec.ObjectSpecification;
 
-import lombok.AccessLevel;
-import lombok.Getter;
 import lombok.val;
 
 
-/**
- * This is populated in two parts.
- *
- * Initially the <tt>specByClassName</tt> map is populated using {@link #put(String, ObjectSpecification)}.
- * This allows {@link #allSpecifications()} to return a list of specs.
- * Later on, {@link #init()} called which populates #classNameBySpecId.
- *
- * Attempting to call {@link #getByObjectType(ObjectSpecId)} before 
- * {@link #init() initialisation} will result in an
- * {@link IllegalStateException}.
- *
- */
 class SpecificationCacheDefault<T extends ObjectSpecification> {
 
     private final Map<String, T> specByClassName = _Maps.newHashMap();
     private final Map<ObjectSpecId, String> classNameBySpecId = _Maps.newHashMap();
     
-    @Getter(AccessLevel.PACKAGE) private boolean initialized = false; //package scoped JUnit support
-
     public T get(final String className) {
         return specByClassName.get(className);
     }
@@ -71,7 +54,6 @@ class SpecificationCacheDefault<T extends ObjectSpecification> {
     }
 
     public void clear() {
-        initialized = false;
         specByClassName.clear();
         classNameBySpecId.clear();
     }
@@ -84,29 +66,10 @@ class SpecificationCacheDefault<T extends ObjectSpecification> {
     }
 
     public T getByObjectType(final ObjectSpecId objectSpecID) {
-        if (!isInitialized()) {
-            throw new IllegalStateException("SpecificationCache by object type has not yet been initialized");
-        }
         final String className = classNameBySpecId.get(objectSpecID);
         return className != null ? specByClassName.get(className) : null;
     }
 
-    void init() {
-        val cachedSpecifications = _Lists.<T>newArrayList();
-        for(;;) {
-            val newSpecifications = snapshotSpecs();
-            newSpecifications.removeAll(cachedSpecifications);
-            if(newSpecifications.isEmpty()) {
-                break;
-            }
-            for (val spec : newSpecifications) {
-                internalPut(spec);
-            }
-            cachedSpecifications.addAll(newSpecifications);
-        }
-        initialized = true;
-    }
-
     private void internalPut(T spec) {
         if(spec==null) {
             return;
diff --git a/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationLoaderDefault.java b/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationLoaderDefault.java
index 751fb93..ecb4dff 100644
--- a/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationLoaderDefault.java
+++ b/core/metamodel/src/main/java/org/apache/isis/metamodel/specloader/SpecificationLoaderDefault.java
@@ -28,13 +28,11 @@ import javax.inject.Inject;
 
 import org.springframework.stereotype.Service;
 
-import org.apache.isis.commons.internal.base._Casts;
 import org.apache.isis.commons.internal.base._Timing;
 import org.apache.isis.commons.internal.collections._Lists;
 import org.apache.isis.config.IsisConfiguration;
 import org.apache.isis.config.registry.IsisBeanTypeRegistry;
 import org.apache.isis.metamodel.facetapi.Facet;
-import org.apache.isis.metamodel.facets.object.objectspecid.ObjectSpecIdFacet;
 import org.apache.isis.metamodel.progmodel.ProgrammingModel;
 import org.apache.isis.metamodel.progmodel.ProgrammingModelService;
 import org.apache.isis.metamodel.progmodels.dflt.ProgrammingModelFacetsJava8;
@@ -130,8 +128,6 @@ public class SpecificationLoaderDefault implements SpecificationLoader {
             log.debug("initialising {}", this);
         }
         
-        val stopWatch = _Timing.now();
-        
         // initialize subcomponents
         programmingModel.init();
         facetProcessor.init();
@@ -184,8 +180,7 @@ public class SpecificationLoaderDefault implements SpecificationLoader {
 
         });
 
-
-        cache.init();
+        val stopWatch = _Timing.now();
 
         val cachedSpecifications = cache.snapshotSpecs();
 
@@ -291,14 +286,10 @@ public class SpecificationLoaderDefault implements SpecificationLoader {
 
     @Override
     public ObjectSpecification lookupBySpecIdElseLoad(ObjectSpecId objectSpecId) {
-        if(!cache.isInitialized()) {
-            throw new IllegalStateException("Internal cache not yet initialized");
-        }
         val spec = cache.getByObjectType(objectSpecId);
         if(spec!=null) {
             return spec;
         }
-
         // fallback
         return loadSpecification(objectSpecId.asString(), IntrospectionState.TYPE_AND_MEMBERS_INTROSPECTED);
     }
@@ -396,35 +387,18 @@ public class SpecificationLoaderDefault implements SpecificationLoader {
 
     private void invalidateCache(final Class<?> cls) {
 
-        if(!cache.isInitialized()) {
-            // could be called by JRebel plugin, before we are up-and-running
-            // just ignore.
-            return;
-        }
-        final Class<?> substitutedType = classSubstitutor.getClass(cls);
-
-        if(substitutedType.isAnonymousClass()) {
-            // JRebel plugin might call us... just ignore 'em.
-            return;
-        }
+        val substitutedType = classSubstitutor.getClass(cls);
 
-        ObjectSpecification spec = loadSpecification(substitutedType, IntrospectionState.TYPE_AND_MEMBERS_INTROSPECTED);
+        ObjectSpecification spec = 
+                loadSpecification(substitutedType, IntrospectionState.TYPE_AND_MEMBERS_INTROSPECTED);
+        
         while(spec != null) {
-            final Class<?> type = spec.getCorrespondingClass();
+            val type = spec.getCorrespondingClass();
             cache.remove(type.getName());
-            if(spec.containsDoOpFacet(ObjectSpecIdFacet.class)) {
-                // umm.  Some specs do not have an ObjectSpecIdFacet...
-                recache(spec);
-            }
             spec = spec.superclass();
         }
     }
 
-
-    private void recache(final ObjectSpecification newSpec) {
-        cache.recache(newSpec);
-    }
-
     // -- DEPS
 
     @Inject private ProgrammingModelService programmingModelService;
diff --git a/core/metamodel/src/test/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefaultTest.java b/core/metamodel/src/test/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefaultTest.java
index 14ac36a..a90a2a8 100644
--- a/core/metamodel/src/test/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefaultTest.java
+++ b/core/metamodel/src/test/java/org/apache/isis/metamodel/specloader/SpecificationCacheDefaultTest.java
@@ -103,14 +103,14 @@ public class SpecificationCacheDefaultTest {
         assertThat(allSpecs.size(), is(2));
     }
 
-    @Test(expected=IllegalStateException.class)
+    @Test
     public void getByObjectType_whenNotSet() {
-        specificationCache.getByObjectType(ObjectSpecId.of("CUS"));
+        assertNull(specificationCache.getByObjectType(ObjectSpecId.of("CUS")));
     }
 
     @Test
     public void getByObjectType_whenSet() {
-        specificationCache.init();
+        
         specificationCache.computeIfAbsent(Customer.class.getName(), __->customerSpec);
         val objectSpec = specificationCache.getByObjectType(ObjectSpecId.of("CUS"));
 
diff --git a/examples/smoketests/src/test/java/org/apache/isis/testdomain/domainmodel/SpecloaderPerformanceTest.java b/examples/smoketests/src/test/java/org/apache/isis/testdomain/domainmodel/SpecloaderPerformanceTest.java
index a488140..73f3ac4 100644
--- a/examples/smoketests/src/test/java/org/apache/isis/testdomain/domainmodel/SpecloaderPerformanceTest.java
+++ b/examples/smoketests/src/test/java/org/apache/isis/testdomain/domainmodel/SpecloaderPerformanceTest.java
@@ -20,12 +20,14 @@ package org.apache.isis.testdomain.domainmodel;
 
 import javax.inject.Inject;
 
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.TestPropertySource;
 
 import org.apache.isis.config.IsisConfiguration;
 import org.apache.isis.config.IsisPresets;
+import org.apache.isis.config.registry.IsisBeanTypeRegistry;
 import org.apache.isis.metamodel.specloader.SpecificationLoader;
 import org.apache.isis.testdomain.Smoketest;
 import org.apache.isis.testdomain.conf.Configuration_headless;
@@ -51,11 +53,20 @@ class SpecloaderPerformanceTest {
     @Inject private IsisConfiguration config;
     @Inject private SpecificationLoader specificationLoader;
     
+    @BeforeAll
+    static void setup() {
+        IsisBeanTypeRegistry.repeatedTesting = true;
+    }
+    
     @Test //under constr.
     void repeatedSpecloading() {
         
-        specificationLoader.shutdown();
-        specificationLoader.init();
+        config.getReflector().getIntrospector().setParallelize(false);
+        
+        for(int i=0; i<1; ++i) {
+            specificationLoader.shutdown();
+            specificationLoader.init();
+        }
     }