From 3fe11d5ca55315412c7928cead0a88ac77f7a52e Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 25 Apr 2021 16:55:43 -0700 Subject: [PATCH 01/12] chore: update asset scheme --- .../org/terasology/gestalt/assets/Asset.java | 37 ++++ .../terasology/gestalt/assets/AssetType.java | 182 +++++++++++++----- 2 files changed, 166 insertions(+), 53 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 4f063b09..67444aac 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -22,6 +22,9 @@ import org.terasology.context.annotation.API; +import java.lang.ref.PhantomReference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.SoftReference; import java.util.Optional; /** @@ -56,6 +59,40 @@ public abstract class Asset { private final DisposalHook disposalHook = new DisposalHook(); private volatile boolean disposed; + protected AssetNode> next; + protected Asset parent; + + protected static class AssetNode> { + public AssetNode(U root,U instance) { + this.reference = new SoftReference<>(instance); + instance.parent = root; + } + SoftReference reference; + AssetNode next; + + protected void clearParent() { + U instance = reference == null ? null: reference.get(); + if(instance != null) { + instance.parent = null; + } + } + + protected void setParent(U parent) { + U instance = reference == null ? null: reference.get(); + if(instance != null) { + instance.parent = parent; + } + } + + protected boolean hasValidAsset() { + U instance = reference == null ? null: reference.get(); + if(instance == null) { + return false; + } + return !instance.isDisposed(); + } + } + /** * The constructor for an asset. It is suggested that implementing classes provide a constructor taking both the urn, and an initial AssetData to load. * diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 074c1217..a46819c6 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -20,14 +20,10 @@ import com.google.common.base.Function; import com.google.common.base.Preconditions; -import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Collections2; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.MapMaker; -import com.google.common.collect.Multimaps; import com.google.common.collect.Sets; import net.jcip.annotations.ThreadSafe; @@ -43,17 +39,21 @@ import java.lang.ref.PhantomReference; import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; +import java.lang.ref.SoftReference; import java.lang.ref.WeakReference; import java.lang.reflect.Type; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.Semaphore; +import java.util.stream.Collectors; /** * AssetType manages all assets of a particular type/class. It provides the ability to resolve and load assets by Urn, and caches assets so that there is only @@ -75,8 +75,7 @@ public final class AssetType, U extends AssetData> implements private final Class assetDataClass; private final AssetFactory factory; private final List> producers = Lists.newCopyOnWriteArrayList(); - private final Map loadedAssets = new MapMaker().concurrencyLevel(4).makeMap(); - private final ListMultimap> instanceAssets = Multimaps.synchronizedListMultimap(ArrayListMultimap.>create()); + private final Map> loadedAssets = new MapMaker().concurrencyLevel(4).makeMap(); // Per-asset locks to deal with situations where multiple threads attempt to obtain or create the same unloaded asset concurrently private final Map locks = new MapMaker().concurrencyLevel(1).makeMap(); @@ -134,12 +133,17 @@ public synchronized void close() { @SuppressWarnings("unchecked") public void processDisposal() { Reference> ref = disposalQueue.poll(); + Set urns = new HashSet<>(); while (ref != null) { AssetReference> assetRef = (AssetReference>) ref; + urns.add(assetRef.parentUrn); assetRef.dispose(); references.remove(assetRef); ref = disposalQueue.poll(); } + for (ResourceUrn urn : urns) { + compactResource(urn); + } } /** @@ -153,23 +157,23 @@ public synchronized boolean isClosed() { * Disposes all assets of this type. */ public synchronized void disposeAll() { - loadedAssets.values().forEach(T::dispose); - - for (WeakReference assetRef : ImmutableList.copyOf(instanceAssets.values())) { - T asset = assetRef.get(); - if (asset != null) { + loadedAssets.values().forEach(k -> { + Asset asset = k.get(); + if(asset != null) { asset.dispose(); + Asset.AssetNode> current = asset.next; + while(current != null){ + SoftReference> target = current.reference; + Asset en = target.get(); + if(en != null) { + en.dispose(); + } + current = current.next; + } } - } + }); + loadedAssets.clear(); processDisposal(); - if (!loadedAssets.isEmpty()) { - logger.error("Assets remained loaded after disposal - {}", loadedAssets.keySet()); - loadedAssets.clear(); - } - if (!instanceAssets.isEmpty()) { - logger.error("Asset instances remained loaded after disposal - {}", instanceAssets.keySet()); - instanceAssets.clear(); - } } /** @@ -181,14 +185,17 @@ public synchronized void disposeAll() { */ public void refresh() { if (!closed) { - for (T asset : loadedAssets.values()) { - if (!followRedirects(asset.getUrn()).equals(asset.getUrn()) || !reloadFromProducers(asset)) { + for (Reference target : loadedAssets.values()) { + T asset = target.get(); + if (asset != null && (!followRedirects(asset.getUrn()).equals(asset.getUrn()) || !reloadFromProducers(asset))) { asset.dispose(); - for (WeakReference instanceRef : ImmutableList.copyOf(instanceAssets.get(asset.getUrn().getInstanceUrn()))) { - T instance = instanceRef.get(); - if (instance != null) { - instance.dispose(); + Asset.AssetNode> current = asset.next; + while (current != null){ + Asset value = current.reference.get(); + if(value != null) { + value.dispose(); } + current = current.next; } } } @@ -267,33 +274,95 @@ public Optional getAsset(ResourceUrn urn) { } /** - * Notifies the asset type when an asset is disposed + * Notifies the asset type when an asset is created * - * @param asset The asset that was disposed. + * @param asset The asset that was created */ - void onAssetDisposed(Asset asset) { - if (asset.getUrn().isInstance()) { - instanceAssets.get(asset.getUrn()).remove(new WeakReference<>(assetClass.cast(asset))); + synchronized void registerAsset(Asset asset, DisposalHook disposer) { + if (closed) { + throw new IllegalStateException("Cannot create asset for disposed asset type: " + assetClass); + } else { + if(addAsset(assetClass.cast(asset))) { + references.add(new AssetReference<>(asset, disposalQueue, disposer)); + } + } + } + + + private boolean addAsset(T targetAsset) { + + ResourceUrn targetUrn = targetAsset.getUrn(); + ResourceUrn parentUrn = targetUrn.getParentUrn(); + + Reference reference = loadedAssets.get(parentUrn); + T current = reference == null ? null: reference.get(); + if(current != null){ + ResourceUrn referenceUrn = current.getUrn(); + if(!referenceUrn.isInstance() && !targetUrn.isInstance()) { + throw new RuntimeException("An already non instance asset has been registered"); + } else if(targetUrn.isInstance()) { // target object is instanced + Asset.AssetNode> next = current.next; + Asset.AssetNode> newNode = new Asset.AssetNode<>(current, targetAsset); + current.next = newNode; + newNode.next = next; + } else { // target object is non instanced so replace instance with + targetAsset.next = new Asset.AssetNode<>(targetAsset, current); + Asset.AssetNode> next = targetAsset.next; + while (next != null) { + next.setParent(targetAsset); // update all parents to point to new target + next = next.next; + } + loadedAssets.put(parentUrn, targetUrn.isInstance() ? new WeakReference(targetAsset) : new SoftReference(targetAsset)); + } } else { - loadedAssets.remove(asset.getUrn()); + loadedAssets.put(parentUrn, targetUrn.isInstance() ? new WeakReference(targetAsset) : new SoftReference(targetAsset)); } + return true; } + /** - * Notifies the asset type when an asset is created + * Notifies the asset type when an asset is disposed * - * @param asset The asset that was created + * @param asset The asset that was disposed. */ - synchronized void registerAsset(Asset asset, DisposalHook disposer) { - if (closed) { - throw new IllegalStateException("Cannot create asset for disposed asset type: " + assetClass); - } else { - if (asset.getUrn().isInstance()) { - instanceAssets.put(asset.getUrn(), new WeakReference<>(assetClass.cast(asset))); + void onAssetDisposed(Asset asset) { + if (!asset.getUrn().isInstance()) { + compactResource(asset.getUrn()); + } + } + + private void compactResource(ResourceUrn target) { + Preconditions.checkArgument(!target.isInstance()); + if(!loadedAssets.containsKey(target)) { + return; + } + Reference reference = loadedAssets.get(target); + Asset current = reference.get(); + if(current == null) { + loadedAssets.remove(target); + return; + } + boolean rootInstanceDisposed = current.isDisposed(); + Asset.AssetNode> node = current.next; + while (node != null) { + Asset.AssetNode> nextAsset = node.next; + while (nextAsset != null && !nextAsset.hasValidAsset()) { + nextAsset = nextAsset.next; + } + node.next = nextAsset; + if(rootInstanceDisposed) { + node.clearParent(); + } + node = nextAsset; + } + if(rootInstanceDisposed) { + if(current.next != null && current.next.reference != null) { + loadedAssets.put(target, (Reference) current.next.reference); } else { - loadedAssets.put(asset.getUrn(), assetClass.cast(asset)); + loadedAssets.remove(target); } - references.add(new AssetReference<>(asset, disposalQueue, disposer)); + } } @@ -362,7 +431,8 @@ public Optional reload(ResourceUrn urn) { return Optional.of(loadAsset(redirectUrn, data.get())); } } - return Optional.ofNullable(loadedAssets.get(redirectUrn)); + Reference reference = loadedAssets.get(redirectUrn); + return Optional.ofNullable(reference == null ? null : reference.get()); }); } catch (PrivilegedActionException e) { if (redirectUrn.equals(urn)) { @@ -382,7 +452,8 @@ public Optional reload(ResourceUrn urn) { */ private Optional getNormalAsset(ResourceUrn urn) { ResourceUrn redirectUrn = followRedirects(urn); - T asset = loadedAssets.get(redirectUrn); + Reference reference = loadedAssets.get(redirectUrn); + T asset = reference == null ? null: reference.get(); if (asset == null) { return reload(redirectUrn); } @@ -503,11 +574,13 @@ private boolean reloadFromProducers(Asset asset) { if (data.isPresent()) { asset.reload(data.get()); - for (WeakReference assetInstanceRef : instanceAssets.get(asset.getUrn().getInstanceUrn())) { - T assetInstance = assetInstanceRef.get(); - if (assetInstance != null) { - assetInstance.reload(data.get()); + Asset.AssetNode> current = asset.next; + while (current != null) { + Asset inst = current.reference.get(); + if(inst != null) { + inst.reload(data.get()); } + current = current.next; } return true; } @@ -529,7 +602,8 @@ public T loadAsset(ResourceUrn urn, U data) { if (urn.isInstance()) { return factory.build(urn, this, data); } else { - T asset = loadedAssets.get(urn); + Reference reference = loadedAssets.get(urn); + T asset = reference == null ? null : reference.get(); if (asset != null) { asset.reload(data); } else { @@ -540,7 +614,8 @@ public T loadAsset(ResourceUrn urn, U data) { try { lock.lock(); if (!closed) { - asset = loadedAssets.get(urn); + reference = loadedAssets.get(urn); + asset = reference == null ? null : reference.get(); if (asset == null) { asset = factory.build(urn, this, data); } else { @@ -581,7 +656,7 @@ public Set getLoadedAssetUrns() { * @return A list of all the loaded assets. */ public Set getLoadedAssets() { - return ImmutableSet.copyOf(loadedAssets.values()); + return ImmutableSet.copyOf(loadedAssets.values().stream().map(Reference::get).filter(Objects::nonNull).collect(Collectors.toSet())); } /** @@ -642,13 +717,14 @@ public String toString() { } } - private static final class AssetReference extends PhantomReference { + public final class AssetReference> extends PhantomReference { private final DisposalHook disposalHook; - + public final ResourceUrn parentUrn; public AssetReference(T asset, ReferenceQueue queue, DisposalHook hook) { super(asset, queue); this.disposalHook = hook; + parentUrn = asset.getUrn().getParentUrn(); } public void dispose() { From fdcb4eb435a7f9a476684aee445e8cc5210a8699 Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 2 May 2021 12:22:44 -0700 Subject: [PATCH 02/12] cleanup --- .../java/org/terasology/gestalt/assets/Asset.java | 8 ++++---- .../org/terasology/gestalt/assets/AssetType.java | 12 +++++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 67444aac..a723db9f 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -54,15 +54,15 @@ @ThreadSafe public abstract class Asset { + AssetNode> next; + Asset parent; + private final ResourceUrn urn; private final AssetType assetType; private final DisposalHook disposalHook = new DisposalHook(); private volatile boolean disposed; - protected AssetNode> next; - protected Asset parent; - - protected static class AssetNode> { + static class AssetNode> { public AssetNode(U root,U instance) { this.reference = new SoftReference<>(instance); instance.parent = root; diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index a46819c6..8a295a6d 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -159,13 +159,13 @@ public synchronized boolean isClosed() { public synchronized void disposeAll() { loadedAssets.values().forEach(k -> { Asset asset = k.get(); - if(asset != null) { + if (asset != null) { asset.dispose(); Asset.AssetNode> current = asset.next; - while(current != null){ + while (current != null) { SoftReference> target = current.reference; Asset en = target.get(); - if(en != null) { + if (en != null) { en.dispose(); } current = current.next; @@ -288,9 +288,7 @@ synchronized void registerAsset(Asset asset, DisposalHook disposer) { } } - private boolean addAsset(T targetAsset) { - ResourceUrn targetUrn = targetAsset.getUrn(); ResourceUrn parentUrn = targetUrn.getParentUrn(); @@ -656,7 +654,7 @@ public Set getLoadedAssetUrns() { * @return A list of all the loaded assets. */ public Set getLoadedAssets() { - return ImmutableSet.copyOf(loadedAssets.values().stream().map(Reference::get).filter(Objects::nonNull).collect(Collectors.toSet())); + return loadedAssets.values().stream().map(Reference::get).filter(Objects::nonNull).collect(Collectors.toSet()); } /** @@ -717,7 +715,7 @@ public String toString() { } } - public final class AssetReference> extends PhantomReference { + private final class AssetReference> extends PhantomReference { private final DisposalHook disposalHook; public final ResourceUrn parentUrn; From 02481af5a3bfe770c2408ddf8410c903674b4e3b Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 2 May 2021 16:31:59 -0700 Subject: [PATCH 03/12] simplify asset logic --- .../org/terasology/gestalt/assets/Asset.java | 65 ++++-- .../terasology/gestalt/assets/AssetType.java | 193 +++++++----------- 2 files changed, 122 insertions(+), 136 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index a723db9f..27eeb8c7 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -17,14 +17,13 @@ package org.terasology.gestalt.assets; import com.google.common.base.Preconditions; - import net.jcip.annotations.ThreadSafe; - +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.terasology.context.annotation.API; -import java.lang.ref.PhantomReference; -import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; +import java.security.PrivilegedActionException; import java.util.Optional; /** @@ -53,39 +52,40 @@ @API @ThreadSafe public abstract class Asset { + private static final Logger logger = LoggerFactory.getLogger(Asset.class); - AssetNode> next; - Asset parent; + AssetNode next; + Asset parent; private final ResourceUrn urn; private final AssetType assetType; private final DisposalHook disposalHook = new DisposalHook(); private volatile boolean disposed; - static class AssetNode> { - public AssetNode(U root,U instance) { + static class AssetNode { + public AssetNode(Asset root, Asset instance) { this.reference = new SoftReference<>(instance); instance.parent = root; } - SoftReference reference; - AssetNode next; + SoftReference> reference; + AssetNode next; protected void clearParent() { - U instance = reference == null ? null: reference.get(); + Asset instance = reference == null ? null: reference.get(); if(instance != null) { instance.parent = null; } } - protected void setParent(U parent) { - U instance = reference == null ? null: reference.get(); + protected void setParent(Asset parent) { + Asset instance = reference == null ? null: reference.get(); if(instance != null) { instance.parent = parent; } } protected boolean hasValidAsset() { - U instance = reference == null ? null: reference.get(); + Asset instance = reference == null ? null: reference.get(); if(instance == null) { return false; } @@ -93,6 +93,17 @@ protected boolean hasValidAsset() { } } + void pushAsset(Asset asset) { + if (next == null) { + Preconditions.checkArgument(!getUrn().isInstance()); + next = new AssetNode<>(this, asset); + } else { + AssetNode node = new AssetNode<>(parent, asset); + node.next = next; + next = node; + } + } + /** * The constructor for an asset. It is suggested that implementing classes provide a constructor taking both the urn, and an initial AssetData to load. * @@ -150,12 +161,30 @@ public final synchronized void reload(T data) { @SuppressWarnings("unchecked") public final > Optional createInstance() { Preconditions.checkState(!disposed); - return (Optional) assetType.createInstance(this); + ResourceUrn instanceUrn = getUrn().getInstanceUrn(); + + Optional> assetCopy = doCreateCopy(instanceUrn, assetType); + if (!assetCopy.isPresent()) { + Optional assetData; + try { + assetData = assetType.fetchAssetData(instanceUrn); + } catch (PrivilegedActionException e) { + logger.error("Failed to load asset '" + urn + "'", e.getCause()); + return Optional.empty(); + } + if (assetData.isPresent()) { + assetCopy = Optional.ofNullable(assetType.loadAsset(instanceUrn, assetData.get())); + } + } + assetCopy.ifPresent(this::pushAsset); + return (Optional) assetCopy; } - final synchronized Optional> createCopy(ResourceUrn copyUrn) { - Preconditions.checkState(!disposed); - return doCreateCopy(copyUrn, assetType); + public final Asset getNormalAsset() { + if(parent == null) { + return this; + } + return parent; } /** diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 8a295a6d..21e58400 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -17,7 +17,6 @@ package org.terasology.gestalt.assets; import android.support.annotation.Nullable; - import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.Collections2; @@ -25,9 +24,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.MapMaker; import com.google.common.collect.Sets; - import net.jcip.annotations.ThreadSafe; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.terasology.context.annotation.API; @@ -40,7 +37,6 @@ import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; -import java.lang.ref.WeakReference; import java.lang.reflect.Type; import java.security.AccessController; import java.security.PrivilegedActionException; @@ -161,7 +157,7 @@ public synchronized void disposeAll() { Asset asset = k.get(); if (asset != null) { asset.dispose(); - Asset.AssetNode> current = asset.next; + Asset.AssetNode current = asset.next; while (current != null) { SoftReference> target = current.reference; Asset en = target.get(); @@ -189,9 +185,9 @@ public void refresh() { T asset = target.get(); if (asset != null && (!followRedirects(asset.getUrn()).equals(asset.getUrn()) || !reloadFromProducers(asset))) { asset.dispose(); - Asset.AssetNode> current = asset.next; + Asset.AssetNode current = asset.next; while (current != null){ - Asset value = current.reference.get(); + Asset value = current.reference.get(); if(value != null) { value.dispose(); } @@ -282,43 +278,13 @@ synchronized void registerAsset(Asset asset, DisposalHook disposer) { if (closed) { throw new IllegalStateException("Cannot create asset for disposed asset type: " + assetClass); } else { - if(addAsset(assetClass.cast(asset))) { - references.add(new AssetReference<>(asset, disposalQueue, disposer)); + if(!asset.getUrn().isInstance()) { + loadedAssets.put(asset.getUrn(), new SoftReference(assetClass.cast(asset))); } + references.add(new AssetReference<>(asset, disposalQueue, disposer)); } } - private boolean addAsset(T targetAsset) { - ResourceUrn targetUrn = targetAsset.getUrn(); - ResourceUrn parentUrn = targetUrn.getParentUrn(); - - Reference reference = loadedAssets.get(parentUrn); - T current = reference == null ? null: reference.get(); - if(current != null){ - ResourceUrn referenceUrn = current.getUrn(); - if(!referenceUrn.isInstance() && !targetUrn.isInstance()) { - throw new RuntimeException("An already non instance asset has been registered"); - } else if(targetUrn.isInstance()) { // target object is instanced - Asset.AssetNode> next = current.next; - Asset.AssetNode> newNode = new Asset.AssetNode<>(current, targetAsset); - current.next = newNode; - newNode.next = next; - } else { // target object is non instanced so replace instance with - targetAsset.next = new Asset.AssetNode<>(targetAsset, current); - Asset.AssetNode> next = targetAsset.next; - while (next != null) { - next.setParent(targetAsset); // update all parents to point to new target - next = next.next; - } - loadedAssets.put(parentUrn, targetUrn.isInstance() ? new WeakReference(targetAsset) : new SoftReference(targetAsset)); - } - } else { - loadedAssets.put(parentUrn, targetUrn.isInstance() ? new WeakReference(targetAsset) : new SoftReference(targetAsset)); - } - return true; - } - - /** * Notifies the asset type when an asset is disposed * @@ -342,9 +308,9 @@ private void compactResource(ResourceUrn target) { return; } boolean rootInstanceDisposed = current.isDisposed(); - Asset.AssetNode> node = current.next; + Asset.AssetNode node = current.next; while (node != null) { - Asset.AssetNode> nextAsset = node.next; + Asset.AssetNode nextAsset = node.next; while (nextAsset != null && !nextAsset.hasValidAsset()) { nextAsset = nextAsset.next; } @@ -373,12 +339,13 @@ private void compactResource(ResourceUrn target) { * * @param urn The urn of the asset to create an instance of * @return An instance of the desired asset + * */ @SuppressWarnings("unchecked") public Optional getInstanceAsset(ResourceUrn urn) { Optional parentAsset = getAsset(urn.getParentUrn()); if (parentAsset.isPresent()) { - return createInstance(parentAsset.get()); + return parentAsset.get().createInstance(); } else { return Optional.empty(); } @@ -391,26 +358,21 @@ public Optional getInstanceAsset(ResourceUrn urn) { * @return The new instance, or {@link Optional#empty} if it could not be created */ Optional createInstance(Asset asset) { - Preconditions.checkArgument(assetClass.isAssignableFrom(asset.getClass())); - Optional> result = asset.createCopy(asset.getUrn().getInstanceUrn()); - if (!result.isPresent()) { - try { - return AccessController.doPrivileged((PrivilegedExceptionAction>) () -> { - for (AssetDataProducer producer : producers) { - Optional data = producer.getAssetData(asset.getUrn()); - if (data.isPresent()) { - return Optional.of(loadAsset(asset.getUrn().getInstanceUrn(), data.get())); - } - } - return Optional.ofNullable(assetClass.cast(result.get())); - }); - } catch (PrivilegedActionException e) { - logger.error("Failed to load asset '" + asset.getUrn().getInstanceUrn() + "'", e.getCause()); - } - } - return Optional.ofNullable(assetClass.cast(result.get())); + return asset.createInstance(); } + + public Optional fetchAssetData(ResourceUrn urn) throws PrivilegedActionException { + return AccessController.doPrivileged((PrivilegedExceptionAction>) () -> { + for (AssetDataProducer producer : producers) { + Optional data = producer.getAssetData(urn); + if (data.isPresent()) { + return data; + } + } + return Optional.empty(); + }); + } /** * Forces a reload of an asset from a data producer, if possible. The resource urn must not be an instance urn (it doesn't make sense to reload an instance by urn). * If there is no available source for the asset (it has no producer) then it will not be reloaded. @@ -422,16 +384,12 @@ public Optional reload(ResourceUrn urn) { Preconditions.checkArgument(!urn.isInstance(), "Cannot reload an asset instance urn"); ResourceUrn redirectUrn = followRedirects(urn); try { - return AccessController.doPrivileged((PrivilegedExceptionAction>) () -> { - for (AssetDataProducer producer : producers) { - Optional data = producer.getAssetData(redirectUrn); - if (data.isPresent()) { - return Optional.of(loadAsset(redirectUrn, data.get())); - } - } - Reference reference = loadedAssets.get(redirectUrn); - return Optional.ofNullable(reference == null ? null : reference.get()); - }); + Optional data = fetchAssetData(urn); + if (data.isPresent()) { + return Optional.of(loadAsset(redirectUrn, data.get())); + } + Reference reference = loadedAssets.get(redirectUrn); + return Optional.ofNullable(reference == null ? null : reference.get()); } catch (PrivilegedActionException e) { if (redirectUrn.equals(urn)) { logger.error("Failed to load asset '{}'", redirectUrn, e.getCause()); @@ -442,6 +400,51 @@ public Optional reload(ResourceUrn urn) { return Optional.empty(); } + /** + * Loads an asset with the given urn and data. If the asset already exists, it is reloaded with the data instead + * + * @param urn The urn of the asset + * @param data The data to load the asset with + * @return The loaded (or reloaded) asset + */ + public T loadAsset(ResourceUrn urn, U data) { + if (urn.isInstance()) { + return factory.build(urn, this, data); + } else { + Reference reference = loadedAssets.get(urn); + T asset = reference == null ? null : reference.get(); + if (asset != null) { + asset.reload(data); + } else { + ResourceLock lock; + synchronized (locks) { + lock = locks.computeIfAbsent(urn, k -> new ResourceLock(urn)); + } + try { + lock.lock(); + if (!closed) { + reference = loadedAssets.get(urn); + asset = reference == null ? null : reference.get(); + if (asset == null) { + asset = factory.build(urn, this, data); + } else { + asset.reload(data); + } + } + synchronized (locks) { + if (lock.unlock()) { + locks.remove(urn); + } + } + } catch (InterruptedException e) { + logger.error("Failed to load asset - interrupted awaiting lock on resource {}", urn); + } + } + + return asset; + } + } + /** * Obtains a non-instance asset * @@ -569,10 +572,9 @@ private boolean reloadFromProducers(Asset asset) { try { for (AssetDataProducer producer : producers) { Optional data = producer.getAssetData(asset.getUrn()); - if (data.isPresent()) { asset.reload(data.get()); - Asset.AssetNode> current = asset.next; + Asset.AssetNode current = asset.next; while (current != null) { Asset inst = current.reference.get(); if(inst != null) { @@ -589,51 +591,6 @@ private boolean reloadFromProducers(Asset asset) { return false; } - /** - * Loads an asset with the given urn and data. If the asset already exists, it is reloaded with the data instead - * - * @param urn The urn of the asset - * @param data The data to load the asset with - * @return The loaded (or reloaded) asset - */ - public T loadAsset(ResourceUrn urn, U data) { - if (urn.isInstance()) { - return factory.build(urn, this, data); - } else { - Reference reference = loadedAssets.get(urn); - T asset = reference == null ? null : reference.get(); - if (asset != null) { - asset.reload(data); - } else { - ResourceLock lock; - synchronized (locks) { - lock = locks.computeIfAbsent(urn, k -> new ResourceLock(urn)); - } - try { - lock.lock(); - if (!closed) { - reference = loadedAssets.get(urn); - asset = reference == null ? null : reference.get(); - if (asset == null) { - asset = factory.build(urn, this, data); - } else { - asset.reload(data); - } - } - synchronized (locks) { - if (lock.unlock()) { - locks.remove(urn); - } - } - } catch (InterruptedException e) { - logger.error("Failed to load asset - interrupted awaiting lock on resource {}", urn); - } - } - - return asset; - } - } - /** * @param urn The urn of the asset to check. Must not be an instance urn * @return Whether an asset is loaded with the given urn From 6302a1f536ce22ca0342b8f888549a886ab7f262 Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 2 May 2021 17:09:50 -0700 Subject: [PATCH 04/12] chore: simplify cleanup logic --- .../org/terasology/gestalt/assets/Asset.java | 14 ----------- .../terasology/gestalt/assets/AssetType.java | 23 ++++++++----------- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 27eeb8c7..25a1edda 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -70,20 +70,6 @@ public AssetNode(Asset root, Asset instance) { SoftReference> reference; AssetNode next; - protected void clearParent() { - Asset instance = reference == null ? null: reference.get(); - if(instance != null) { - instance.parent = null; - } - } - - protected void setParent(Asset parent) { - Asset instance = reference == null ? null: reference.get(); - if(instance != null) { - instance.parent = parent; - } - } - protected boolean hasValidAsset() { Asset instance = reference == null ? null: reference.get(); if(instance == null) { diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 21e58400..36555594 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -301,13 +301,19 @@ private void compactResource(ResourceUrn target) { if(!loadedAssets.containsKey(target)) { return; } + Reference reference = loadedAssets.get(target); Asset current = reference.get(); - if(current == null) { + if(current != null && current.isDisposed() && current.next != null) { + logger.warn("non instanced asset is disposed with instances. instances will become orphaned."); + } + + if(current == null || current.isDisposed()) { + // disposing of a non instanced asset will orphan the instanced assets. loadedAssets.remove(target); return; } - boolean rootInstanceDisposed = current.isDisposed(); + Asset.AssetNode node = current.next; while (node != null) { Asset.AssetNode nextAsset = node.next; @@ -315,21 +321,11 @@ private void compactResource(ResourceUrn target) { nextAsset = nextAsset.next; } node.next = nextAsset; - if(rootInstanceDisposed) { - node.clearParent(); - } node = nextAsset; } - if(rootInstanceDisposed) { - if(current.next != null && current.next.reference != null) { - loadedAssets.put(target, (Reference) current.next.reference); - } else { - loadedAssets.remove(target); - } - - } } + /** * Creates and returns an instance of an asset, if possible. The following methods are used to create the copy, in order, with the first technique to succeeed used: *
    @@ -670,6 +666,7 @@ public boolean unlock() { public String toString() { return "lock(" + urn + ")"; } + } private final class AssetReference> extends PhantomReference { From 76e3b08fb36a009c6704e49bddc2914c5eeaedf6 Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sat, 15 May 2021 21:20:12 -0700 Subject: [PATCH 05/12] hide implementation details for instances --- .../org/terasology/gestalt/assets/Asset.java | 77 +++++++++++++++++-- .../terasology/gestalt/assets/AssetType.java | 44 +++-------- 2 files changed, 81 insertions(+), 40 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 25a1edda..dca8465f 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -17,13 +17,17 @@ package org.terasology.gestalt.assets; import com.google.common.base.Preconditions; +import com.google.common.collect.Iterables; +import com.sun.org.apache.xml.internal.dtm.ref.EmptyIterator; import net.jcip.annotations.ThreadSafe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.terasology.context.annotation.API; -import java.lang.ref.SoftReference; +import java.lang.ref.WeakReference; import java.security.PrivilegedActionException; +import java.util.Collections; +import java.util.Iterator; import java.util.Optional; /** @@ -54,21 +58,22 @@ public abstract class Asset { private static final Logger logger = LoggerFactory.getLogger(Asset.class); - AssetNode next; - Asset parent; + private AssetNode next; + private Asset parent; private final ResourceUrn urn; private final AssetType assetType; private final DisposalHook disposalHook = new DisposalHook(); private volatile boolean disposed; - static class AssetNode { + private static class AssetNode { + WeakReference> reference; + AssetNode next; + public AssetNode(Asset root, Asset instance) { - this.reference = new SoftReference<>(instance); + this.reference = new WeakReference<>(instance); instance.parent = root; } - SoftReference> reference; - AssetNode next; protected boolean hasValidAsset() { Asset instance = reference == null ? null: reference.get(); @@ -79,7 +84,8 @@ protected boolean hasValidAsset() { } } - void pushAsset(Asset asset) { + + private void pushAsset(Asset asset) { if (next == null) { Preconditions.checkArgument(!getUrn().isInstance()); next = new AssetNode<>(this, asset); @@ -90,6 +96,53 @@ void pushAsset(Asset asset) { } } + protected Iterable>> instances() { + if (this.parent != null) { + return parent.instances(); + } + AssetNode rootNode = this.next; + if(rootNode == null) { + return Collections::emptyIterator; + } + return () -> new Iterator<>() { + AssetNode node = null; + + @Override + public boolean hasNext() { + if (node == null) { + return true; + } + return node.next != null; + } + + @Override + public WeakReference> next() { + if (node == null) { + node = rootNode.next; + return rootNode.reference; + } + node = node.next; + return node.reference; + } + }; + } + + protected void cleanup() { + if(this.parent != null) { + parent.cleanup(); + return; + } + AssetNode node = next; + while (node != null) { + AssetNode nextAsset = node.next; + while (nextAsset != null && !nextAsset.hasValidAsset()) { + nextAsset = nextAsset.next; + } + node.next = nextAsset; + node = nextAsset; + } + } + /** * The constructor for an asset. It is suggested that implementing classes provide a constructor taking both the urn, and an initial AssetData to load. * @@ -181,6 +234,14 @@ public final synchronized void dispose() { disposed = true; assetType.onAssetDisposed(this); disposalHook.dispose(); + if(parent == null) { + for (WeakReference> inst : this.instances()) { + Asset current = inst.get(); + if (current != null) { + current.dispose(); + } + } + } } } diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 36555594..2a58b8ca 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.MapMaker; import com.google.common.collect.Sets; @@ -37,6 +38,7 @@ import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; +import java.lang.ref.WeakReference; import java.lang.reflect.Type; import java.security.AccessController; import java.security.PrivilegedActionException; @@ -157,15 +159,6 @@ public synchronized void disposeAll() { Asset asset = k.get(); if (asset != null) { asset.dispose(); - Asset.AssetNode current = asset.next; - while (current != null) { - SoftReference> target = current.reference; - Asset en = target.get(); - if (en != null) { - en.dispose(); - } - current = current.next; - } } }); loadedAssets.clear(); @@ -185,13 +178,11 @@ public void refresh() { T asset = target.get(); if (asset != null && (!followRedirects(asset.getUrn()).equals(asset.getUrn()) || !reloadFromProducers(asset))) { asset.dispose(); - Asset.AssetNode current = asset.next; - while (current != null){ - Asset value = current.reference.get(); - if(value != null) { - value.dispose(); + for(WeakReference> it :asset.instances()) { + Asset instance = it.get(); + if(instance != null) { + instance.dispose(); } - current = current.next; } } } @@ -304,7 +295,7 @@ private void compactResource(ResourceUrn target) { Reference reference = loadedAssets.get(target); Asset current = reference.get(); - if(current != null && current.isDisposed() && current.next != null) { + if(current != null && current.isDisposed() && Iterables.isEmpty(current.instances())) { logger.warn("non instanced asset is disposed with instances. instances will become orphaned."); } @@ -313,16 +304,7 @@ private void compactResource(ResourceUrn target) { loadedAssets.remove(target); return; } - - Asset.AssetNode node = current.next; - while (node != null) { - Asset.AssetNode nextAsset = node.next; - while (nextAsset != null && !nextAsset.hasValidAsset()) { - nextAsset = nextAsset.next; - } - node.next = nextAsset; - node = nextAsset; - } + current.cleanup(); } @@ -570,13 +552,11 @@ private boolean reloadFromProducers(Asset asset) { Optional data = producer.getAssetData(asset.getUrn()); if (data.isPresent()) { asset.reload(data.get()); - Asset.AssetNode current = asset.next; - while (current != null) { - Asset inst = current.reference.get(); - if(inst != null) { - inst.reload(data.get()); + for(WeakReference> it :asset.instances()) { + Asset instance = it.get(); + if(instance != null) { + instance.reload(data.get()); } - current = current.next; } return true; } From 2da83d1d0dfe93ad4f6f40c2fbedd0ea2acb0207 Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sat, 15 May 2021 21:23:07 -0700 Subject: [PATCH 06/12] cleanup imports --- .../src/main/java/org/terasology/gestalt/assets/Asset.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index dca8465f..9e16e457 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -17,8 +17,6 @@ package org.terasology.gestalt.assets; import com.google.common.base.Preconditions; -import com.google.common.collect.Iterables; -import com.sun.org.apache.xml.internal.dtm.ref.EmptyIterator; import net.jcip.annotations.ThreadSafe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From 6d23d42f06c020099cee648a9e992a477bbe3198 Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Mon, 17 May 2021 20:34:43 -0700 Subject: [PATCH 07/12] core type argument --- .../src/main/java/org/terasology/gestalt/assets/Asset.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 9e16e457..7d4b7f51 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -102,7 +102,7 @@ protected Iterable>> instances() { if(rootNode == null) { return Collections::emptyIterator; } - return () -> new Iterator<>() { + return () -> new Iterator>>() { AssetNode node = null; @Override From 186ad9ca06489aafef3e81ef72fb78644c2d6ec3 Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 13 Jun 2021 18:18:46 -0700 Subject: [PATCH 08/12] chore: remove custom linkedlist --- .../org/terasology/gestalt/assets/Asset.java | 87 ++++++------------- 1 file changed, 26 insertions(+), 61 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 7d4b7f51..30198095 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -17,6 +17,7 @@ package org.terasology.gestalt.assets; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; import net.jcip.annotations.ThreadSafe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,6 +27,7 @@ import java.security.PrivilegedActionException; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Optional; /** @@ -56,7 +58,7 @@ public abstract class Asset { private static final Logger logger = LoggerFactory.getLogger(Asset.class); - private AssetNode next; + private final List>> instances; private Asset parent; private final ResourceUrn urn; @@ -64,33 +66,14 @@ public abstract class Asset { private final DisposalHook disposalHook = new DisposalHook(); private volatile boolean disposed; - private static class AssetNode { - WeakReference> reference; - AssetNode next; - - public AssetNode(Asset root, Asset instance) { - this.reference = new WeakReference<>(instance); - instance.parent = root; - } - - protected boolean hasValidAsset() { - Asset instance = reference == null ? null: reference.get(); - if(instance == null) { - return false; - } - return !instance.isDisposed(); - } - } - - - private void pushAsset(Asset asset) { - if (next == null) { + private void appendInstance(Asset asset) { + if (parent == null) { Preconditions.checkArgument(!getUrn().isInstance()); - next = new AssetNode<>(this, asset); + asset.parent = this; + instances.add(new WeakReference<>(asset)); } else { - AssetNode node = new AssetNode<>(parent, asset); - node.next = next; - next = node; + asset.parent = parent; + parent.instances.add(new WeakReference<>(asset)); } } @@ -98,31 +81,7 @@ protected Iterable>> instances() { if (this.parent != null) { return parent.instances(); } - AssetNode rootNode = this.next; - if(rootNode == null) { - return Collections::emptyIterator; - } - return () -> new Iterator>>() { - AssetNode node = null; - - @Override - public boolean hasNext() { - if (node == null) { - return true; - } - return node.next != null; - } - - @Override - public WeakReference> next() { - if (node == null) { - node = rootNode.next; - return rootNode.reference; - } - node = node.next; - return node.reference; - } - }; + return instances; } protected void cleanup() { @@ -130,14 +89,13 @@ protected void cleanup() { parent.cleanup(); return; } - AssetNode node = next; - while (node != null) { - AssetNode nextAsset = node.next; - while (nextAsset != null && !nextAsset.hasValidAsset()) { - nextAsset = nextAsset.next; + Iterator>> iter = instances.iterator(); + while(iter.hasNext()) { + WeakReference> reference = iter.next(); + Asset inst = reference.get(); + if(inst == null || inst.isDisposed()) { + iter.remove(); } - node.next = nextAsset; - node = nextAsset; } } @@ -150,6 +108,7 @@ protected void cleanup() { protected Asset(ResourceUrn urn, AssetType assetType) { Preconditions.checkNotNull(urn); Preconditions.checkNotNull(assetType); + instances = urn.isInstance() ? Collections.emptyList(): Lists.newArrayList(); this.urn = urn; this.assetType = assetType; assetType.registerAsset(this, disposalHook); @@ -161,8 +120,14 @@ protected Asset(ResourceUrn urn, AssetType assetType) { * this would prevent it being garbage collected. It must be a static inner class, or not contained in the asset class * (or an anonymous class defined in a static context). A warning will be logged if this is not the case. */ - protected void setDisposableResource(DisposableResource resource) { - this.disposalHook.setDisposableResource(resource); + public Asset(ResourceUrn urn, AssetType assetType, DisposableResource resource) { + Preconditions.checkNotNull(urn); + Preconditions.checkNotNull(assetType); + instances = urn.isInstance() ? Collections.emptyList(): Lists.newArrayList(); + this.urn = urn; + this.assetType = assetType; + disposalHook.setDisposableResource(resource); + assetType.registerAsset(this, disposalHook); } /** @@ -213,7 +178,7 @@ public final > Optional createInstance() { assetCopy = Optional.ofNullable(assetType.loadAsset(instanceUrn, assetData.get())); } } - assetCopy.ifPresent(this::pushAsset); + assetCopy.ifPresent(this::appendInstance); return (Optional) assetCopy; } From 652f1a9783757520ffb9dbb6ed77b00355eab56e Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Tue, 13 Jul 2021 21:29:08 -0700 Subject: [PATCH 09/12] chore: minor correct logic to handle asset cleanup and add javadocs --- .../org/terasology/gestalt/assets/Asset.java | 36 ++++++------- .../terasology/gestalt/assets/AssetType.java | 53 ++++++++----------- 2 files changed, 38 insertions(+), 51 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index 30198095..ef72c092 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -1,19 +1,5 @@ -/* - * Copyright 2019 MovingBlocks - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 package org.terasology.gestalt.assets; import com.google.common.base.Preconditions; @@ -77,16 +63,20 @@ private void appendInstance(Asset asset) { } } - protected Iterable>> instances() { + /** + * return all the instances that are associated with this type of asset. + * @return instances + */ + protected final Iterable>> instances() { if (this.parent != null) { return parent.instances(); } return instances; } - protected void cleanup() { - if(this.parent != null) { - parent.cleanup(); + private void compactInstances() { + if (this.parent != null) { + parent.compactInstances(); return; } Iterator>> iter = instances.iterator(); @@ -182,6 +172,11 @@ public final > Optional createInstance() { return (Optional) assetCopy; } + /** + * return the non-instanced version of this asset. if the asset is already the normal + * type then it returns itself. + * @return non-instanced version of this Asset + */ public final Asset getNormalAsset() { if(parent == null) { return this; @@ -194,6 +189,7 @@ public final Asset getNormalAsset() { */ public final synchronized void dispose() { if (!disposed) { + compactInstances(); disposed = true; assetType.onAssetDisposed(this); disposalHook.dispose(); diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 2a58b8ca..507fb001 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -1,19 +1,5 @@ -/* - * Copyright 2019 MovingBlocks - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 package org.terasology.gestalt.assets; import android.support.annotation.Nullable; @@ -21,7 +7,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.MapMaker; import com.google.common.collect.Sets; @@ -140,7 +125,7 @@ public void processDisposal() { ref = disposalQueue.poll(); } for (ResourceUrn urn : urns) { - compactResource(urn); + disposeAsset(urn); } } @@ -269,7 +254,7 @@ synchronized void registerAsset(Asset asset, DisposalHook disposer) { if (closed) { throw new IllegalStateException("Cannot create asset for disposed asset type: " + assetClass); } else { - if(!asset.getUrn().isInstance()) { + if (!asset.getUrn().isInstance()) { loadedAssets.put(asset.getUrn(), new SoftReference(assetClass.cast(asset))); } references.add(new AssetReference<>(asset, disposalQueue, disposer)); @@ -283,28 +268,34 @@ synchronized void registerAsset(Asset asset, DisposalHook disposer) { */ void onAssetDisposed(Asset asset) { if (!asset.getUrn().isInstance()) { - compactResource(asset.getUrn()); + disposeAsset(asset.getUrn()); } } - private void compactResource(ResourceUrn target) { + /** + * dispose asset and remove loaded asset from {@link #loadedAssets} + * @param target urn to free + */ + private void disposeAsset(ResourceUrn target) { Preconditions.checkArgument(!target.isInstance()); - if(!loadedAssets.containsKey(target)) { + if (!loadedAssets.containsKey(target)) { return; } Reference reference = loadedAssets.get(target); Asset current = reference.get(); - if(current != null && current.isDisposed() && Iterables.isEmpty(current.instances())) { - logger.warn("non instanced asset is disposed with instances. instances will become orphaned."); - } - - if(current == null || current.isDisposed()) { - // disposing of a non instanced asset will orphan the instanced assets. + if (current == null) { + loadedAssets.remove(target); + } else if (current.isDisposed()) { + for (WeakReference> it : current.instances()) { + Asset instance = it.get(); + if (instance != null && !instance.isDisposed()) { + logger.warn("non instanced asset is disposed with instances. instances will become orphaned."); + break; + } + } loadedAssets.remove(target); - return; } - current.cleanup(); } @@ -649,7 +640,7 @@ public String toString() { } - private final class AssetReference> extends PhantomReference { + private static final class AssetReference> extends PhantomReference { private final DisposalHook disposalHook; public final ResourceUrn parentUrn; From 41f75bc3ae71ce1cd713c78d8fb1187142e9006d Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Mon, 26 Jul 2021 21:59:31 -0700 Subject: [PATCH 10/12] chore: update layout --- .../org/terasology/gestalt/assets/Asset.java | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index ef72c092..a8e5170d 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -52,6 +52,22 @@ public abstract class Asset { private final DisposalHook disposalHook = new DisposalHook(); private volatile boolean disposed; + + /** + * The constructor for an asset. It is suggested that implementing classes provide a constructor taking both the urn, and an initial AssetData to load. + * + * @param urn The urn identifying the asset. + * @param assetType The asset type this asset belongs to. + */ + protected Asset(ResourceUrn urn, AssetType assetType) { + Preconditions.checkNotNull(urn); + Preconditions.checkNotNull(assetType); + instances = urn.isInstance() ? Collections.emptyList() : Lists.newArrayList(); + this.urn = urn; + this.assetType = assetType; + assetType.registerAsset(this, disposalHook); + } + private void appendInstance(Asset asset) { if (parent == null) { Preconditions.checkArgument(!getUrn().isInstance()); @@ -89,21 +105,6 @@ private void compactInstances() { } } - /** - * The constructor for an asset. It is suggested that implementing classes provide a constructor taking both the urn, and an initial AssetData to load. - * - * @param urn The urn identifying the asset. - * @param assetType The asset type this asset belongs to. - */ - protected Asset(ResourceUrn urn, AssetType assetType) { - Preconditions.checkNotNull(urn); - Preconditions.checkNotNull(assetType); - instances = urn.isInstance() ? Collections.emptyList(): Lists.newArrayList(); - this.urn = urn; - this.assetType = assetType; - assetType.registerAsset(this, disposalHook); - } - /** * set a resource handler so the disposable hook can clean up resources not managed by the JVM * @param resource A resource to close when disposing this class. The resource must not have a reference to this asset - @@ -177,8 +178,8 @@ public final > Optional createInstance() { * type then it returns itself. * @return non-instanced version of this Asset */ - public final Asset getNormalAsset() { - if(parent == null) { + public final Asset getConcreteAsset() { + if (parent == null) { return this; } return parent; From 29e14cde21e9f677001ed00220c44184fb11c7eb Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 1 Aug 2021 08:32:07 -0700 Subject: [PATCH 11/12] chore: update assettype --- .../org/terasology/gestalt/assets/Asset.java | 6 +++--- .../org/terasology/gestalt/assets/AssetType.java | 16 ++-------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java index a8e5170d..bb31275b 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/Asset.java @@ -175,10 +175,11 @@ public final > Optional createInstance() { /** * return the non-instanced version of this asset. if the asset is already the normal - * type then it returns itself. + * type then it returns itself. instanced assets are temporary copies from the normal loaded instances. + * * @return non-instanced version of this Asset */ - public final Asset getConcreteAsset() { + public final Asset getNormalAsset() { if (parent == null) { return this; } @@ -192,7 +193,6 @@ public final synchronized void dispose() { if (!disposed) { compactInstances(); disposed = true; - assetType.onAssetDisposed(this); disposalHook.dispose(); if(parent == null) { for (WeakReference> inst : this.instances()) { diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 507fb001..7444ee9a 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -115,14 +115,13 @@ public synchronized void close() { */ @SuppressWarnings("unchecked") public void processDisposal() { - Reference> ref = disposalQueue.poll(); Set urns = new HashSet<>(); - while (ref != null) { + Reference> ref = null; + while ((ref = disposalQueue.poll()) != null) { AssetReference> assetRef = (AssetReference>) ref; urns.add(assetRef.parentUrn); assetRef.dispose(); references.remove(assetRef); - ref = disposalQueue.poll(); } for (ResourceUrn urn : urns) { disposeAsset(urn); @@ -261,17 +260,6 @@ synchronized void registerAsset(Asset asset, DisposalHook disposer) { } } - /** - * Notifies the asset type when an asset is disposed - * - * @param asset The asset that was disposed. - */ - void onAssetDisposed(Asset asset) { - if (!asset.getUrn().isInstance()) { - disposeAsset(asset.getUrn()); - } - } - /** * dispose asset and remove loaded asset from {@link #loadedAssets} * @param target urn to free From 6f22479f38c1af1ebf1613f2ab119cbc5430a17c Mon Sep 17 00:00:00 2001 From: Michael Pollind Date: Sun, 21 Nov 2021 20:51:14 -0800 Subject: [PATCH 12/12] chore: correctly handled disposed asset --- .../main/java/org/terasology/gestalt/assets/AssetType.java | 7 +++++-- .../src/test/java/virtualModules/test/stubs/book/Book.java | 3 +-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java index 7444ee9a..eea38773 100644 --- a/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java +++ b/gestalt-asset-core/src/main/java/org/terasology/gestalt/assets/AssetType.java @@ -301,7 +301,7 @@ private void disposeAsset(ResourceUrn target) { @SuppressWarnings("unchecked") public Optional getInstanceAsset(ResourceUrn urn) { Optional parentAsset = getAsset(urn.getParentUrn()); - if (parentAsset.isPresent()) { + if (parentAsset.isPresent() && !parentAsset.get().isDisposed()) { return parentAsset.get().createInstance(); } else { return Optional.empty(); @@ -411,10 +411,13 @@ public T loadAsset(ResourceUrn urn, U data) { private Optional getNormalAsset(ResourceUrn urn) { ResourceUrn redirectUrn = followRedirects(urn); Reference reference = loadedAssets.get(redirectUrn); - T asset = reference == null ? null: reference.get(); + T asset = reference == null ? null : reference.get(); if (asset == null) { return reload(redirectUrn); } + if (asset.isDisposed()) { + return Optional.empty(); + } return Optional.ofNullable(asset); } diff --git a/gestalt-asset-core/src/test/java/virtualModules/test/stubs/book/Book.java b/gestalt-asset-core/src/test/java/virtualModules/test/stubs/book/Book.java index d42d00c0..24e27272 100644 --- a/gestalt-asset-core/src/test/java/virtualModules/test/stubs/book/Book.java +++ b/gestalt-asset-core/src/test/java/virtualModules/test/stubs/book/Book.java @@ -35,8 +35,7 @@ public class Book extends Asset { private ImmutableList lines = ImmutableList.of(); public Book(ResourceUrn urn, BookData data, AssetType type) { - super(urn, type); - setDisposableResource(new DisposalAction(urn)); + super(urn, type, new DisposalAction(urn)); reload(data); }