Pipeline Optimisation : Optimising getPath Method

helix-core/src/main/java/org/apache/helix/PropertyPathBuilder.java

@@ -19,8 +19,10 @@
  * under the License.
  */
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -49,8 +51,8 @@
 public class PropertyPathBuilder {
   private static Logger logger = LoggerFactory.getLogger(PropertyPathBuilder.class);
 
-  static final Map<PropertyType, Map<Integer, String>> templateMap =
-      new HashMap<PropertyType, Map<Integer, String>>();
+  static final Map<PropertyType, Map<Integer, PathTemplate>> templateMap =
+      new HashMap<PropertyType, Map<Integer, PathTemplate>>();
   @Deprecated // typeToClassMapping is not being used anywhere
   static final Map<PropertyType, Class<? extends HelixProperty>> typeToClassMapping =
       new HashMap<PropertyType, Class<? extends HelixProperty>>();
@@ -180,15 +182,67 @@ public class PropertyPathBuilder {
     addEntry(PropertyType.JOB_CONTEXT, 3, "/{clusterName}/PROPERTYSTORE"
         + TaskConstants.REBALANCER_CONTEXT_ROOT + "/{workflowName}" + "_" + "{jobName}/Context");
   }
-  static Pattern pattern = Pattern.compile("(\\{.+?\\})");
+
+  /**
+   * Pre-parsed path template for efficient path construction.
+   * Parses template strings once at initialization to avoid repeated regex operations.
+   */
+  static class PathTemplate {
+    private final String[] parts;
+
+    /**
+     * Parse a template string like "/{clusterName}/INSTANCES/{instanceName}"
+     * into reusable parts: ["/", "/INSTANCES/", ""]
+     */
+    PathTemplate(String template) {
+      List<String> partsList = new ArrayList<>();
+      int lastEnd = 0;
+
+      // Find all {placeholders} and split the template into static parts
+      int start = template.indexOf('{');
+      while (start != -1) {
+        partsList.add(template.substring(lastEnd, start));
+        int end = template.indexOf('}', start);
+        if (end == -1) {
+          logger.error("Malformed template: missing closing brace in " + template);
+          break;
+        }
+        lastEnd = end + 1;
+        start = template.indexOf('{', lastEnd);
+      }
+      partsList.add(template.substring(lastEnd));
+
+      this.parts = partsList.toArray(new String[0]);
+    }
+
+    /**
+     * Build path by concatenating static parts with parameters.
+     */
+    String buildPath(String clusterName, String... keys) {
+      StringBuilder sb = new StringBuilder(128);
+      sb.append(parts[0]).append(clusterName);
+
+      for (int i = 0; i < keys.length; i++) {
+        if (i + 1 < parts.length) {
+          sb.append(parts[i + 1]);
+        }
+        sb.append(keys[i]);
+      }
+
+      if (keys.length + 1 < parts.length) {
+        sb.append(parts[keys.length + 1]);
+      }
+
+      return sb.toString();
+    }
+  }
 
   private static void addEntry(PropertyType type, int numKeys, String template) {
     if (!templateMap.containsKey(type)) {
-      templateMap.put(type, new HashMap<Integer, String>());
+      templateMap.put(type, new HashMap<Integer, PathTemplate>());
     }
-    logger.trace("Adding template for type:" + type.getType() + " arguments:" + numKeys
-        + " template:" + template);
-    templateMap.get(type).put(numKeys, template);
+    logger.trace("Adding template for type:" + type.getType() + " arguments:" + numKeys + " template:" + template);
+    templateMap.get(type).put(numKeys, new PathTemplate(template));
   }
 
   /**
@@ -204,33 +258,22 @@ public static String getPath(PropertyType type, String clusterName, String... ke
       return null;
     }
     if (keys == null) {
-      keys = new String[] {};
+      keys = new String[]{};
     }
-    String template = null;
+
+    PathTemplate template = null;
     if (templateMap.containsKey(type)) {
-      // keys.length+1 since we add clusterName
       template = templateMap.get(type).get(keys.length + 1);
     }
 
     String result = null;
-
     if (template != null) {
-      result = template;
-      Matcher matcher = pattern.matcher(template);
-      int count = 0;
-      while (matcher.find()) {
-        count = count + 1;
-        String var = matcher.group();
-        if (count == 1) {
-          result = result.replace(var, clusterName);
-        } else {
-          result = result.replace(var, keys[count - 2]);
-        }
-      }
+      result = template.buildPath(clusterName, keys);
     }
+
     if (result == null || result.indexOf('{') > -1 || result.indexOf('}') > -1) {
-      logger.warn("Unable to instantiate template:" + template + " using clusterName:" + clusterName
-          + " and keys:" + Arrays.toString(keys));
+      logger.warn("Unable to instantiate template for type:" + type + " using clusterName:" + clusterName + " and keys:"
+          + Arrays.toString(keys));
     }
     return result;
   }

helix-core/src/test/java/org/apache/helix/TestPropertyPathBuilder.java

@@ -42,10 +42,8 @@ public void testGetPath() {
 
     actual = PropertyPathBuilder.instanceTaskCurrentState("test_cluster", "instanceName1");
     AssertJUnit.assertEquals(actual, "/test_cluster/INSTANCES/instanceName1/TASKCURRENTSTATES");
-    actual =
-        PropertyPathBuilder.instanceTaskCurrentState("test_cluster", "instanceName1", "sessionId");
-    AssertJUnit
-        .assertEquals(actual, "/test_cluster/INSTANCES/instanceName1/TASKCURRENTSTATES/sessionId");
+    actual = PropertyPathBuilder.instanceTaskCurrentState("test_cluster", "instanceName1", "sessionId");
+    AssertJUnit.assertEquals(actual, "/test_cluster/INSTANCES/instanceName1/TASKCURRENTSTATES/sessionId");
 
     actual = PropertyPathBuilder.instanceCustomizedState("test_cluster", "instanceName1");
     AssertJUnit.assertEquals(actual, "/test_cluster/INSTANCES/instanceName1/CUSTOMIZEDSTATES");
@@ -60,4 +58,80 @@ public void testGetPath() {
     actual = PropertyPathBuilder.clusterStatus("test_cluster");
     Assert.assertEquals(actual, "/test_cluster/STATUS/CLUSTER/test_cluster");
   }
+
+  /**
+   * Test the core getPath() method with the optimized PathTemplate implementation.
+   * Tests paths with varying numbers of parameters (0-4 keys).
+   */
+  @Test
+  public void testGetPathWithPropertyType() {
+    String actual;
+
+    actual =
+        PropertyPathBuilder.getPath(PropertyType.CURRENTSTATES, "MyCluster", "instance1", "session123", "resource1");
+    Assert.assertEquals(actual, "/MyCluster/INSTANCES/instance1/CURRENTSTATES/session123/resource1");
+
+    actual =
+        PropertyPathBuilder.getPath(PropertyType.CURRENTSTATES, "MyCluster", "instance1", "session123", "resource1",
+            "bucket1");
+    Assert.assertEquals(actual, "/MyCluster/INSTANCES/instance1/CURRENTSTATES/session123/resource1/bucket1");
+
+    actual =
+        PropertyPathBuilder.getPath(PropertyType.CUSTOMIZEDSTATES, "MyCluster", "instance1", "MyState", "MyResource");
+    Assert.assertEquals(actual, "/MyCluster/INSTANCES/instance1/CUSTOMIZEDSTATES/MyState/MyResource");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.CUSTOMIZEDVIEW, "MyCluster");
+    Assert.assertEquals(actual, "/MyCluster/CUSTOMIZEDVIEW");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.CUSTOMIZEDVIEW, "MyCluster", "customType");
+    Assert.assertEquals(actual, "/MyCluster/CUSTOMIZEDVIEW/customType");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.CUSTOMIZEDVIEW, "MyCluster", "customType", "resource1");
+    Assert.assertEquals(actual, "/MyCluster/CUSTOMIZEDVIEW/customType/resource1");
+
+    // Test that getPath() works correctly for basic cases
+    actual = PropertyPathBuilder.getPath(PropertyType.LIVEINSTANCES, "MyCluster", "node1");
+    Assert.assertEquals(actual, "/MyCluster/LIVEINSTANCES/node1");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.IDEALSTATES, "MyCluster", "MyResource");
+    Assert.assertEquals(actual, "/MyCluster/IDEALSTATES/MyResource");
+
+    actual =
+        PropertyPathBuilder.getPath(PropertyType.STATUSUPDATES, "MyCluster", "inst1", "sess1", "subpath", "record1");
+    Assert.assertEquals(actual, "/MyCluster/INSTANCES/inst1/STATUSUPDATES/sess1/subpath/record1");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.ERRORS, "MyCluster", "inst1", "sess1", "subpath", "record1");
+    Assert.assertEquals(actual, "/MyCluster/INSTANCES/inst1/ERRORS/sess1/subpath/record1");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.TASKCURRENTSTATES, "MyCluster", "inst1", "sess1", "resource1",
+        "bucket1");
+    Assert.assertEquals(actual, "/MyCluster/INSTANCES/inst1/TASKCURRENTSTATES/sess1/resource1/bucket1");
+
+    actual = PropertyPathBuilder.getPath(PropertyType.CONFIGS, "MyCluster", "PARTICIPANT", "instance1");
+    Assert.assertEquals(actual, "/MyCluster/CONFIGS/PARTICIPANT/instance1");
+  }
+
+  /**
+   * Test edge cases and special scenarios
+   */
+  @Test
+  public void testGetPathEdgeCases() {
+    String actual;
+
+    // Test with null clusterName - should return null and log warning
+    actual = PropertyPathBuilder.getPath(PropertyType.IDEALSTATES, null);
+    Assert.assertNull(actual);
+
+    // Test with null keys array - should work (treated as empty array)
+    actual = PropertyPathBuilder.getPath(PropertyType.LIVEINSTANCES, "TestCluster", (String[]) null);
+    Assert.assertEquals(actual, "/TestCluster/LIVEINSTANCES");
+
+    // Test with cluster name containing special characters
+    actual = PropertyPathBuilder.getPath(PropertyType.IDEALSTATES, "test-cluster_123");
+    Assert.assertEquals(actual, "/test-cluster_123/IDEALSTATES");
+
+    // Test with resource name containing special characters
+    actual = PropertyPathBuilder.getPath(PropertyType.IDEALSTATES, "cluster", "resource-name_123.test");
+    Assert.assertEquals(actual, "/cluster/IDEALSTATES/resource-name_123.test");
+  }
 }