issue 34046 velocity memory & sync issues

dotCMS/src/main/java/com/dotcms/rendering/velocity/services/DotResourceLoader.java

@@ -5,18 +5,44 @@
 import com.dotmarketing.util.Config;
 import com.dotmarketing.util.Logger;
 import com.dotmarketing.util.UtilMethods;
+import com.google.common.util.concurrent.Striped;
 import java.io.InputStream;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
 import org.apache.commons.collections.ExtendedProperties;
 import org.apache.velocity.exception.ResourceNotFoundException;
 import org.apache.velocity.runtime.resource.Resource;
 import org.apache.velocity.runtime.resource.loader.ResourceLoader;
 
 public class DotResourceLoader extends ResourceLoader {
 
-
     private static DotResourceLoader instance;
 
-    private static boolean useCache = Config.getBooleanProperty("VELOCITY_CACHING_ON", true);
+    private static final boolean useCache = Config.getBooleanProperty("VELOCITY_CACHING_ON", true);
+
+    /**
+     * Timeout in milliseconds for acquiring the resource generation lock.
+     * If a resource takes longer than this to generate, waiting threads will timeout.
+     * Configurable via VELOCITY_RESOURCE_LOAD_TIMEOUT_MS property.
+     */
+    private static final long RESOURCE_LOAD_TIMEOUT_MS =
+            Config.getLongProperty("VELOCITY_RESOURCE_LOAD_TIMEOUT_MS", 15000L); // 15 seconds default
+
+    /**
+     * Number of lock stripes for resource generation.
+     * Higher values reduce contention but use more memory.
+     * Configurable via VELOCITY_RESOURCE_LOAD_STRIPES property.
+     */
+    private static final int RESOURCE_LOAD_STRIPES =
+            Config.getIntProperty("VELOCITY_RESOURCE_LOAD_STRIPES", 64);
+
+    /**
+     * Striped lock to prevent duplicate resource generation.
+     * Only one thread can generate a resource for a given key at a time.
+     * Other threads waiting for the same key will wait until generation completes or timeout.
+     */
+    private static final Striped<Lock> resourceLock = Striped.lazyWeakLock(RESOURCE_LOAD_STRIPES);
+
     public DotResourceLoader() {
         super();
     }
@@ -28,58 +54,75 @@ public InputStream getResourceStream(final String filePath) throws ResourceNotFo
             throw new ResourceNotFoundException("cannot find resource");
         }
 
+        final VelocityResourceKey key = new VelocityResourceKey(filePath);
 
+        Logger.debug(this, "DotResourceLoader:\t: " + key);
 
-        synchronized (filePath.intern()) {
-
-            VelocityResourceKey key = new VelocityResourceKey(filePath);
-
-            Logger.debug(this, "DotResourceLoader:\t: " + key);
-
-            try {
-                switch (key.type) {
-                    case CONTAINER: {
-                        return new ContainerLoader().writeObject(key);
-                    }
-                    case TEMPLATE: {
-                        return new TemplateLoader().writeObject(key);
-                    }
-                    case CONTENT: {
-                        return new ContentletLoader().writeObject(key);
-                    }
-                    case FIELD: {
-                        return new FieldLoader().writeObject(key);
-                    }
-                    case CONTENT_TYPE: {
-                        return new ContentTypeLoader().writeObject(key);
-                    }
-                    case SITE: {
-                        return new SiteLoader().writeObject(key);
-                    }
-                    case HTMLPAGE: {
-                        return new PageLoader().writeObject(key);
-                    }
-                    case VELOCITY_MACROS: {
-                        return VTLLoader.instance().writeObject(key);
-                    }
-                    case VTL: {
-                        return VTLLoader.instance().writeObject(key);
-                    }
-                    case VELOCITY_LEGACY_VL: {
-                        return VTLLoader.instance().writeObject(key);
-                    }
-                    default: {
-                        return IncludeLoader.instance().writeObject(key);
-                    }
-                }
+        // Use striped lock to prevent duplicate resource generation for the same key.
+        // This prevents the "thundering herd" problem where multiple threads try to
+        // generate the same expensive resource simultaneously.
+        final Lock lock = resourceLock.get(key.path);
+        boolean hasLock = false;
+
+        try {
+            hasLock = lock.tryLock(RESOURCE_LOAD_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+
+            if (!hasLock) {
+                Logger.warn(this, "Timeout waiting for resource generation lock: " + key.path +
+                        " after " + RESOURCE_LOAD_TIMEOUT_MS + "ms");
+                throw new ResourceNotFoundException(
+                        "Timeout waiting for velocity resource generation: " + key.path);
+            }
 
-            } catch (Exception e) {
-                Logger.warn(this, "filePath: " + filePath + ", msg:" + e.getMessage(), e);
-                CacheLocator.getVeloctyResourceCache().addMiss(key.path);
-                throw new ResourceNotFoundException("Cannot parse velocity file : " + key.path, e);
+            return loadResource(key);
+
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new ResourceNotFoundException("Interrupted while waiting for resource: " + key.path, e);
+        } catch (ResourceNotFoundException e) {
+            throw e;
+        } catch (Exception e) {
+            Logger.warn(this, "filePath: " + filePath + ", msg:" + e.getMessage(), e);
+            CacheLocator.getVeloctyResourceCache().addMiss(key.path);
+            throw new ResourceNotFoundException("Cannot parse velocity file : " + key.path, e);
+        } finally {
+            if (hasLock) {
+                lock.unlock();
             }
         }
+    }
 
+    /**
+     * Loads the velocity resource based on its type.
+     * This method is called while holding the resource lock to prevent duplicate generation.
+     *
+     * @param key the velocity resource key
+     * @return the resource as an InputStream
+     * @throws Exception if resource loading fails
+     */
+    private InputStream loadResource(final VelocityResourceKey key) throws Exception {
+        switch (key.type) {
+            case CONTAINER:
+                return new ContainerLoader().writeObject(key);
+            case TEMPLATE:
+                return new TemplateLoader().writeObject(key);
+            case CONTENT:
+                return new ContentletLoader().writeObject(key);
+            case FIELD:
+                return new FieldLoader().writeObject(key);
+            case CONTENT_TYPE:
+                return new ContentTypeLoader().writeObject(key);
+            case SITE:
+                return new SiteLoader().writeObject(key);
+            case HTMLPAGE:
+                return new PageLoader().writeObject(key);
+            case VELOCITY_MACROS:
+            case VTL:
+            case VELOCITY_LEGACY_VL:
+                return VTLLoader.instance().writeObject(key);
+            default:
+                return IncludeLoader.instance().writeObject(key);
+        }
     }
 
     /*

dotCMS/src/main/java/com/dotcms/rendering/velocity/services/PageRenderUtil.java

@@ -59,7 +59,6 @@
 import com.liferay.util.StringPool;
 import io.vavr.control.Try;
 import java.io.Serializable;
-import java.io.StringWriter;
 import java.text.SimpleDateFormat;
 import java.time.Instant;
 import java.util.Collection;
@@ -83,6 +82,14 @@ public class PageRenderUtil implements Serializable {
     public static String CONTAINER_UUID_PREFIX = "uuid-";
     private static final long serialVersionUID = 1L;
 
+    /**
+     * ThreadLocal StringBuilder to avoid allocating new StringBuilder/StringWriter objects
+     * in the asString() method. This reduces GC pressure during page rendering as asString()
+     * is called for every page load to generate Velocity variable assignments.
+     */
+    private static final ThreadLocal<StringBuilder> STRING_BUILDER =
+            ThreadLocal.withInitial(() -> new StringBuilder(2048));
+
     private final PermissionAPI permissionAPI = APILocator.getPermissionAPI();
     private final MultiTreeAPI multiTreeAPI = APILocator.getMultiTreeAPI();
     private final ContentletAPI contentletAPI = APILocator.getContentletAPI();
@@ -746,18 +753,24 @@ public Context addAll(Context incoming) {
     /**
      * Return the Velocity code to set the Page's variables as: contentletList, contentType,
      * totalSize, etc.
+     * <p>
+     * Uses a ThreadLocal StringBuilder to minimize object allocation during page rendering.
      *
-     * @return
+     * @return The Velocity code as a String
      */
     public String asString() {
-
-        final StringWriter s = new StringWriter();
-        for (String key : this.contextMap.keySet()) {
-            s.append("#set($").append(key.replace(":", "")).append("=").append(new StringifyObject(this.contextMap.get(key)).from()).append(')');
+        final StringBuilder sb = STRING_BUILDER.get();
+        sb.setLength(0);  // Reset without reallocation
+
+        for (final Map.Entry<String, Object> entry : this.contextMap.entrySet()) {
+            sb.append("#set($")
+              .append(entry.getKey().replace(":", ""))
+              .append('=')
+              .append(new StringifyObject(entry.getValue()).from())
+              .append(')');
         }
 
-        return s.toString();
-
+        return sb.toString();
     }
 
     /**

dotCMS/src/main/java/com/dotcms/rendering/velocity/services/StringifyObject.java

@@ -1,14 +1,32 @@
 package com.dotcms.rendering.velocity.services;
 
 
-
-import java.io.StringWriter;
 import java.text.SimpleDateFormat;
 import java.util.Collection;
 import java.util.Date;
 import java.util.Iterator;
 
+/**
+ * Utility class to convert objects to their string representation for Velocity context.
+ * Uses ThreadLocal buffers to minimize object allocation during page rendering.
+ */
 final class StringifyObject {
+
+    /**
+     * ThreadLocal StringBuilder pool to avoid allocating new StringWriter/StringBuilder
+     * for each stringify operation. This significantly reduces GC pressure during
+     * page rendering where many objects need to be stringified.
+     */
+    private static final ThreadLocal<StringBuilder> BUFFER =
+            ThreadLocal.withInitial(() -> new StringBuilder(256));
+
+    /**
+     * ThreadLocal SimpleDateFormat to avoid creating expensive formatter objects.
+     * SimpleDateFormat is not thread-safe, so ThreadLocal is required.
+     */
+    private static final ThreadLocal<SimpleDateFormat> DATE_FORMAT =
+            ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"));
+
     final String stringified;
 
     public StringifyObject(final Object o) {
@@ -37,62 +55,67 @@ else if(o instanceof String) {
     }
 
 
+    /**
+     * Gets the ThreadLocal StringBuilder, resetting it for reuse.
+     * @return A clean StringBuilder ready for use
+     */
+    private static StringBuilder getBuffer() {
+        final StringBuilder sb = BUFFER.get();
+        sb.setLength(0);  // Reset without reallocation
+        return sb;
+    }
+
     private String stringifyObject(final String[] str) {
-        StringWriter sw = new StringWriter();
-        sw.append('[');
+        final StringBuilder sb = getBuffer();
+        sb.append('[');
         for (int i = 0; i < str.length; i++) {
-            sw.append('"')
+            sb.append('"')
                 .append(str[i])
-                .append("\"");
+                .append('"');
             if (i != str.length - 1) {
-                sw.append(",");
+                sb.append(',');
             }
         }
-        sw.append(']');
-        return  sw.toString();
+        sb.append(']');
+        return sb.toString();
     }
 
-    private String  stringifyObject(final Collection co) {
-        StringWriter sw = new StringWriter();
-
-        sw.append('[');
-        Iterator<Object> it = co.iterator();
+    private String stringifyObject(final Collection<?> co) {
+        final StringBuilder sb = getBuffer();
+        sb.append('[');
+        final Iterator<?> it = co.iterator();
         while (it.hasNext()) {
-            Object obj = it.next();
-            sw.append('"')
+            final Object obj = it.next();
+            sb.append('"')
                 .append(obj.toString())
-                .append("\"");
-
-            if(it.hasNext()) {
-                sw.append(",");
+                .append('"');
+            if (it.hasNext()) {
+                sb.append(',');
             }
         }
-        sw.append(']');
-        return  sw.toString();
+        sb.append(']');
+        return sb.toString();
     }
 
-    private String  stringifyObject(final Boolean o) {
-
-        return ((Boolean)o).toString();
+    private String stringifyObject(final Boolean o) {
+        return o.toString();
     }
 
-    private String  stringifyObject(final String x) {
-        StringWriter sw = new StringWriter();
-
-            sw.append('"');
-            sw.append(x.toString().replace("\"", "`"));
-            sw.append('"');
-            return  sw.toString();
-
+    private String stringifyObject(final String x) {
+        final StringBuilder sb = getBuffer();
+        sb.append('"');
+        sb.append(x.replace("\"", "`"));
+        sb.append('"');
+        return sb.toString();
     }
 
-    private String  stringifyObject(final Date x) {
-        StringWriter sw = new StringWriter();
-        String d = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS").format(x);
-        sw.append('"');
-        sw.append(d);
-        sw.append('"');
-        return  sw.toString();
+    private String stringifyObject(final Date x) {
+        final StringBuilder sb = getBuffer();
+        final String d = DATE_FORMAT.get().format(x);
+        sb.append('"');
+        sb.append(d);
+        sb.append('"');
+        return sb.toString();
     }