From e30bcbf1ba63f8291a2d441604f06cfc902e8e8c Mon Sep 17 00:00:00 2001 From: Astrash Date: Tue, 24 Oct 2023 13:26:35 +1100 Subject: [PATCH 1/6] Improve image caching options --- .../terra/addons/image/ImageLibraryAddon.java | 5 ++ .../ImageLibraryPackConfigTemplate.java | 41 ++++++++++++ .../addons/image/config/image/ImageCache.java | 62 ++++++++++++------- .../addons/image/image/SuppliedImage.java | 27 ++++++++ 4 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java create mode 100644 common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/image/SuppliedImage.java diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/ImageLibraryAddon.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/ImageLibraryAddon.java index fdcbc701d..64d0149b8 100644 --- a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/ImageLibraryAddon.java +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/ImageLibraryAddon.java @@ -6,6 +6,7 @@ import java.util.function.Supplier; import com.dfsek.terra.addons.image.config.ColorLoader; import com.dfsek.terra.addons.image.config.ColorLoader.ColorString; +import com.dfsek.terra.addons.image.config.ImageLibraryPackConfigTemplate; import com.dfsek.terra.addons.image.config.noisesampler.ChannelNoiseSamplerTemplate; import com.dfsek.terra.addons.image.config.noisesampler.DistanceTransformNoiseSamplerTemplate; import com.dfsek.terra.addons.image.config.image.ImageTemplate; @@ -52,6 +53,10 @@ public class ImageLibraryAddon implements AddonInitializer { .getHandler(FunctionalEventHandler.class) .register(addon, ConfigPackPreLoadEvent.class) .priority(10) + .then(event -> { + ImageLibraryPackConfigTemplate config = event.loadTemplate(new ImageLibraryPackConfigTemplate()); + event.getPack().getContext().put(config); + }) .then(event -> { ConfigPack pack = event.getPack(); CheckedRegistry>> imageRegistry = pack.getOrCreateRegistry(IMAGE_REGISTRY_KEY); diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java new file mode 100644 index 000000000..929a073e6 --- /dev/null +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java @@ -0,0 +1,41 @@ +package com.dfsek.terra.addons.image.config; + +import com.dfsek.tectonic.api.config.template.ConfigTemplate; + +import com.dfsek.tectonic.api.config.template.annotations.Default; +import com.dfsek.tectonic.api.config.template.annotations.Description; +import com.dfsek.tectonic.api.config.template.annotations.Value; + +import com.dfsek.terra.api.properties.Properties; + +public class ImageLibraryPackConfigTemplate implements ConfigTemplate, Properties { + // TODO - These would be better as plugin wide config parameters in config.yml + + @Value("images.cache.load-on-use") + @Description("If set to true, images will load into memory upon use rather than on pack load.") + @Default + private boolean loadOnUse = false; + + @Value("images.cache.unload-on-timeout") + @Description("If set to true, images will be removed from memory if not used after a timeout, otherwise images will stay loaded in memory. " + + "Trades decreased memory consumption when not performing image reads for extra processing time required to perform cache lookups.") + @Default + private boolean unloadOnTimeout = false; + + @Value("images.cache.timeout") + @Description("How many seconds to keep images loaded in the image cache for if images.cache.unload-on-timeout is enabled.") + @Default + private int cacheTimeout = 300; + + public boolean loadOnUse() { + return loadOnUse; + } + + public boolean unloadOnTimeout() { + return unloadOnTimeout; + } + + public int getCacheTimeout() { + return cacheTimeout; + } +} diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java index 5f96ea390..565feca49 100644 --- a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java @@ -3,44 +3,62 @@ package com.dfsek.terra.addons.image.config.image; import javax.imageio.ImageIO; import java.io.FileNotFoundException; import java.io.IOException; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import com.dfsek.terra.addons.image.config.ImageLibraryPackConfigTemplate; import com.dfsek.terra.addons.image.image.BufferedImageWrapper; import com.dfsek.terra.addons.image.image.Image; +import com.dfsek.terra.addons.image.image.SuppliedImage; import com.dfsek.terra.api.config.ConfigPack; import com.dfsek.terra.api.config.Loader; import com.dfsek.terra.api.properties.Properties; +import com.dfsek.terra.api.util.generic.Lazy; + +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; + /* * Cache prevents configs from loading the same image multiple times into memory */ -record ImageCache(ConcurrentHashMap map) implements Properties { +record ImageCache(LoadingCache cache) implements Properties { public static Image load(String path, ConfigPack pack, Loader files) throws IOException { - ImageCache cache; + ImageLibraryPackConfigTemplate config = pack.getContext().get(ImageLibraryPackConfigTemplate.class); + ImageCache images; if(!pack.getContext().has(ImageCache.class)) { - cache = new ImageCache(new ConcurrentHashMap<>()); - pack.getContext().put(cache); - } else { - cache = pack.getContext().get(ImageCache.class); + System.out.println("Initializing new image cache"); + var cacheBuilder = Caffeine.newBuilder(); + if (config.unloadOnTimeout()) cacheBuilder.expireAfterAccess(config.getCacheTimeout(), TimeUnit.SECONDS); + images = new ImageCache(cacheBuilder.build(s -> loadImage(path, files))); + pack.getContext().put(images); + } else images = pack.getContext().get(ImageCache.class); + + if (config.loadOnUse()) { + if(config.unloadOnTimeout()) { // Grab directly from cache if images are to unload on timeout + return new SuppliedImage(() -> images.cache.get(path)); + } else { + // If images do not time out, image can be lazily loaded once instead of performing cache lookups for each image operation + Lazy lazyImage = Lazy.lazy(() -> images.cache.get(path)); + return new SuppliedImage(lazyImage::value); + } } - if(cache.map.containsKey(path)) { - return cache.map.get(path); - } else { - try { - BufferedImageWrapper image = new BufferedImageWrapper(ImageIO.read(files.get(path))); - cache.map.put(path, image); - return image; - } catch(IllegalArgumentException e) { - throw new IllegalArgumentException("Unable to load image (image might be too large?)", e); - } catch(IOException e) { - if(e instanceof FileNotFoundException) { - // Rethrow using nicer message - throw new IOException("Unable to load image: No such file or directory: " + path, e); - } - throw new IOException("Unable to load image", e); + return images.cache.get(path); + } + + private static Image loadImage(String path, Loader files) throws IOException { + System.out.println("Loading image: " + path); + try { + return new BufferedImageWrapper(ImageIO.read(files.get(path))); + } catch(IllegalArgumentException e) { + throw new IllegalArgumentException("Unable to load image (image might be too large?)", e); + } catch(IOException e) { + if(e instanceof FileNotFoundException) { + // Rethrow using nicer message + throw new IOException("Unable to load image: No such file or directory: " + path, e); } + throw new IOException("Unable to load image", e); } } } diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/image/SuppliedImage.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/image/SuppliedImage.java new file mode 100644 index 000000000..99fcbfcd3 --- /dev/null +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/image/SuppliedImage.java @@ -0,0 +1,27 @@ +package com.dfsek.terra.addons.image.image; + +import java.util.function.Supplier; + +public class SuppliedImage implements Image { + + private final Supplier imageSupplier; + + public SuppliedImage(Supplier imageSupplier) { + this.imageSupplier = imageSupplier; + } + + @Override + public int getRGB(int x, int y) { + return imageSupplier.get().getRGB(x, y); + } + + @Override + public int getWidth() { + return imageSupplier.get().getWidth(); + } + + @Override + public int getHeight() { + return imageSupplier.get().getHeight(); + } +} From cb6ecff113223b7bada797709089d4bc2c329cff Mon Sep 17 00:00:00 2001 From: Astrash Date: Tue, 24 Oct 2023 13:29:53 +1100 Subject: [PATCH 2/6] Remove sout --- .../com/dfsek/terra/addons/image/config/image/ImageCache.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java index 565feca49..c554b9c6a 100644 --- a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java @@ -27,7 +27,6 @@ record ImageCache(LoadingCache cache) implements Properties { ImageLibraryPackConfigTemplate config = pack.getContext().get(ImageLibraryPackConfigTemplate.class); ImageCache images; if(!pack.getContext().has(ImageCache.class)) { - System.out.println("Initializing new image cache"); var cacheBuilder = Caffeine.newBuilder(); if (config.unloadOnTimeout()) cacheBuilder.expireAfterAccess(config.getCacheTimeout(), TimeUnit.SECONDS); images = new ImageCache(cacheBuilder.build(s -> loadImage(path, files))); @@ -48,7 +47,6 @@ record ImageCache(LoadingCache cache) implements Properties { } private static Image loadImage(String path, Loader files) throws IOException { - System.out.println("Loading image: " + path); try { return new BufferedImageWrapper(ImageIO.read(files.get(path))); } catch(IllegalArgumentException e) { From 3f485b1825abd16b39a86b84ccf858ab4e1da702 Mon Sep 17 00:00:00 2001 From: Astrash Date: Tue, 24 Oct 2023 13:30:09 +1100 Subject: [PATCH 3/6] Add to unload on timeout description --- .../addons/image/config/ImageLibraryPackConfigTemplate.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java index 929a073e6..7d2cd5b58 100644 --- a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java @@ -18,7 +18,7 @@ public class ImageLibraryPackConfigTemplate implements ConfigTemplate, Propertie @Value("images.cache.unload-on-timeout") @Description("If set to true, images will be removed from memory if not used after a timeout, otherwise images will stay loaded in memory. " + - "Trades decreased memory consumption when not performing image reads for extra processing time required to perform cache lookups.") + "Trades decreased memory consumption when not performing any image reads for a period of time for extra processing time required to perform cache lookups.") @Default private boolean unloadOnTimeout = false; From 148b8dfe35039812994d5257e3ae9511c45af75c Mon Sep 17 00:00:00 2001 From: Astrash Date: Tue, 24 Oct 2023 13:34:10 +1100 Subject: [PATCH 4/6] Bump image lib patch version --- common/addons/library-image/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/addons/library-image/build.gradle.kts b/common/addons/library-image/build.gradle.kts index 6f5d40074..fe8706428 100644 --- a/common/addons/library-image/build.gradle.kts +++ b/common/addons/library-image/build.gradle.kts @@ -1,4 +1,4 @@ -version = version("1.0.0") +version = version("1.0.1") dependencies { compileOnlyApi(project(":common:addons:manifest-addon-loader")) From cd8605850f8708edbadb84ecb2bbcc258c74742c Mon Sep 17 00:00:00 2001 From: Astrash Date: Tue, 24 Oct 2023 13:47:25 +1100 Subject: [PATCH 5/6] Replace unload-on-timeout with timeout > 0 --- .../config/ImageLibraryPackConfigTemplate.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java index 7d2cd5b58..b3a78caed 100644 --- a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/ImageLibraryPackConfigTemplate.java @@ -16,23 +16,19 @@ public class ImageLibraryPackConfigTemplate implements ConfigTemplate, Propertie @Default private boolean loadOnUse = false; - @Value("images.cache.unload-on-timeout") - @Description("If set to true, images will be removed from memory if not used after a timeout, otherwise images will stay loaded in memory. " + - "Trades decreased memory consumption when not performing any image reads for a period of time for extra processing time required to perform cache lookups.") - @Default - private boolean unloadOnTimeout = false; - @Value("images.cache.timeout") - @Description("How many seconds to keep images loaded in the image cache for if images.cache.unload-on-timeout is enabled.") + @Description("How many seconds to keep images loaded in the image cache for. " + + "If set to a number greater than 0, images will be removed from memory if not used after the timeout, otherwise images will stay loaded in memory. " + + "Setting the timeout to greater than 0 will trade decreased memory consumption when not performing any image reads for a period of time for extra processing time required to perform cache lookups.") @Default - private int cacheTimeout = 300; + private int cacheTimeout = 0; public boolean loadOnUse() { return loadOnUse; } public boolean unloadOnTimeout() { - return unloadOnTimeout; + return cacheTimeout > 0; } public int getCacheTimeout() { From 0149a29b04f2871c723a598612b4b0bf60f35155 Mon Sep 17 00:00:00 2001 From: Astrash Date: Tue, 24 Oct 2023 14:08:56 +1100 Subject: [PATCH 6/6] Load correct path --- .../com/dfsek/terra/addons/image/config/image/ImageCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java index c554b9c6a..7558d4bfa 100644 --- a/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java +++ b/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java @@ -29,7 +29,7 @@ record ImageCache(LoadingCache cache) implements Properties { if(!pack.getContext().has(ImageCache.class)) { var cacheBuilder = Caffeine.newBuilder(); if (config.unloadOnTimeout()) cacheBuilder.expireAfterAccess(config.getCacheTimeout(), TimeUnit.SECONDS); - images = new ImageCache(cacheBuilder.build(s -> loadImage(path, files))); + images = new ImageCache(cacheBuilder.build(s -> loadImage(s, files))); pack.getContext().put(images); } else images = pack.getContext().get(ImageCache.class);