Changeset 17593 in josm

Timestamp:

2021-03-20T11:50:06+01:00 (4 years ago)

Author:

simon04

Message:

see #20613 - Split KeyCondition/KeyRegexpCondition

Rewrite patterns for prefix/substring/suffix checks.

5.75->2.93% of allocations in MapCSSTagCheckerPerformanceTest.testCity relate to Pattern matching.

Location:

trunk

Files:

• resources/data/validator/deprecated.mapcss (modified) (1 diff)
• resources/data/validator/unnecessary.mapcss (modified) (1 diff)
• src/org/openstreetmap/josm/gui/mappaint/mapcss/ConditionFactory.java (modified) (7 diffs)
• src/org/openstreetmap/josm/gui/mappaint/mapcss/MapCSSRuleIndex.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/gui/mappaint/mapcss/KeyConditionTest.java (modified) (3 diffs)

trunk/resources/data/validator/deprecated.mapcss

@@ -1473 +1473 @@
 /* #17504 */
 *[is_in],
-node[/^is_in:.*$/],
-way[/^is_in:.*$/] {
+node[/^is_in:$/],
+way[/^is_in:$/] {
   throwWarning: tr("{0} is deprecated", "{0.key}");
   group: tr("deprecated tagging");

trunk/resources/data/validator/unnecessary.mapcss

@@ -155 +155 @@
 
 /* #2760 */
-*[/^(gpx|gpxx|gpxd):/] {
+*[/^gpx:/],
+*[/^gpxx:/],
+*[/^gpxd:/] {
   throwWarning: tr("{0} should not be uploaded", "{0.key}");
   group: tr("unnecessary tag");

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/ConditionFactory.java

@@ -105 +105 @@
         switch (context) {
         case PRIMITIVE:
-            return new KeyCondition(k, not, matchType);
+            if (KeyMatchType.REGEX == matchType && k.matches("[A-Za-z0-9:_-]+")) {
+                // optimization: using String.contains avoids allocating a Matcher
+                return new KeyCondition(k, not, KeyMatchType.ANY_CONTAINS);
+            } else if (KeyMatchType.REGEX == matchType && k.matches("\\^[A-Za-z0-9:_-]+")) {
+                // optimization: using String.startsWith avoids allocating a Matcher
+                return new KeyCondition(k.substring(1), not, KeyMatchType.ANY_STARTS_WITH);
+            } else if (KeyMatchType.REGEX == matchType && k.matches("[A-Za-z0-9:_-]+\\$")) {
+                // optimization: using String.endsWith avoids allocating a Matcher
+                return new KeyCondition(k.substring(0, k.length() - 1), not, KeyMatchType.ANY_ENDS_WITH);
+            } else if (matchType == KeyMatchType.REGEX) {
+                return new KeyRegexpCondition(Pattern.compile(k), not);
+            } else {
+                return new KeyCondition(k, not, matchType);
+            }
         case LINK:
             if (matchType != null)
@@ -489 +502 @@
          * The key needs to match the given regular expression.
          */
-        REGEX
+        REGEX,
+        /**
+         * The key needs to contain the given label as substring.
+         */
+        ANY_CONTAINS,
+        /**
+         * The key needs to start with the given label.
+         */
+        ANY_STARTS_WITH,
+        /**
+         * The key needs to end with the given label.
+         */
+        ANY_ENDS_WITH,
     }
 
@@ -510 +535 @@
      *                   LINK:       not supported
      * </pre>
+     * @see KeyRegexpCondition
      */
     public static class KeyCondition implements Condition, ToTagConvertable {
@@ -526 +552 @@
          */
         public final KeyMatchType matchType;
-        /**
-         * A predicate used to match a the regexp against the key. Only used if the match type is regexp.
-         */
-        public final Predicate<String> containsPattern;
 
         /**
@@ -538 +560 @@
          */
         public KeyCondition(String label, boolean negateResult, KeyMatchType matchType) {
+            CheckParameterUtil.ensureThat(matchType != KeyMatchType.REGEX, "Use KeyPatternCondition");
             this.label = label;
             this.negateResult = negateResult;
             this.matchType = matchType == null ? KeyMatchType.EQ : matchType;
-            this.containsPattern = KeyMatchType.REGEX == matchType
-                    ? Pattern.compile(label).asPredicate()
-                    : null;
         }
 
         @Override
         public boolean applies(Environment e) {
-            switch(e.getContext()) {
-            case PRIMITIVE:
-                switch (matchType) {
+            CheckParameterUtil.ensureThat(!e.isLinkContext(), "Illegal state: KeyCondition not supported in LINK context");
+            switch (matchType) {
                 case TRUE:
                     return e.osm.isKeyTrue(label) ^ negateResult;
                 case FALSE:
                     return e.osm.isKeyFalse(label) ^ negateResult;
-                case REGEX:
-                    return e.osm.keys().anyMatch(containsPattern) ^ negateResult;
+                case ANY_CONTAINS:
+                case ANY_STARTS_WITH:
+                case ANY_ENDS_WITH:
+                    return e.osm.keys().anyMatch(keyPredicate()) ^ negateResult;
                 default:
                     return e.osm.hasKey(label) ^ negateResult;
-                }
-            case LINK:
-                Utils.ensure(false, "Illegal state: KeyCondition not supported in LINK context");
-                return false;
-            default: throw new AssertionError();
-            }
+            }
+        }
+
+        private Predicate<String> keyPredicate() {
+            switch (matchType) {
+                case ANY_CONTAINS:
+                    return key -> key.contains(label);
+                case ANY_STARTS_WITH:
+                    return key -> key.startsWith(label);
+                case ANY_ENDS_WITH:
+                    return key -> key.endsWith(label);
+                default:
+                    return null;
+            }
+        }
+
+        /**
+         * Get the matched key and the corresponding value.
+         * <p>
+         * WARNING: This ignores {@link #negateResult}.
+         * @param p The primitive to get the value from.
+         * @return The tag.
+         */
+        @Override
+        public Tag asTag(OsmPrimitive p) {
+            String key = label;
+            Predicate<String> keyPredicate = keyPredicate();
+            if (keyPredicate != null) {
+                key = p.keys().filter(keyPredicate).findAny().orElse(key);
+            }
+            return new Tag(key, p.get(key));
+        }
+
+        @Override
+        public String toString() {
+            return '[' + (negateResult ? "!" : "") + label + ']';
+        }
+    }
+
+    /**
+     * KeyPatternCondition represents a conditions matching keys based on a pattern.
+     */
+    public static class KeyRegexpCondition implements Condition, ToTagConvertable {
+
+        /**
+         * A predicate used to match a the regexp against the key. Only used if the match type is regexp.
+         */
+        public final Pattern pattern;
+        /**
+         * If we should negate the result of the match.
+         */
+        public final boolean negateResult;
+
+        /**
+         * Creates a new KeyPatternCondition
+         * @param pattern The regular expression for matching keys.
+         * @param negateResult If we should negate the result.
+         */
+        public KeyRegexpCondition(Pattern pattern, boolean negateResult) {
+            this.negateResult = negateResult;
+            this.pattern = pattern;
+        }
+
+        @Override
+        public boolean applies(Environment e) {
+            CheckParameterUtil.ensureThat(!e.isLinkContext(), "Illegal state: KeyCondition not supported in LINK context");
+            return e.osm.keys().anyMatch(pattern.asPredicate()) ^ negateResult;
         }
 
@@ -578 +660 @@
         @Override
         public Tag asTag(OsmPrimitive p) {
-            String key = label;
-            if (KeyMatchType.REGEX == matchType) {
-                key = p.keys().filter(containsPattern).findAny().orElse(key);
-            }
+            String key = p.keys().filter(pattern.asPredicate()).findAny().orElse(pattern.pattern());;
             return new Tag(key, p.get(key));
         }
@@ -587 +666 @@
         @Override
         public String toString() {
-            return '[' + (negateResult ? "!" : "") + label + ']';
+            return '[' + (negateResult ? "!" : "") + pattern + ']';
         }
     }

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/MapCSSRuleIndex.java

@@ -16 +16 @@
 import org.openstreetmap.josm.data.osm.Tagged;
 import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.KeyCondition;
-import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.KeyMatchType;
 import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.KeyValueCondition;
 import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.SimpleKeyValueCondition;
@@ -190 +189 @@
             if (c instanceof KeyCondition) {
                 KeyCondition keyCondition = (KeyCondition) c;
-                if (!keyCondition.negateResult && conditionRequiresKeyPresence(keyCondition.matchType)) {
+                if (!keyCondition.negateResult) {
                     key = keyCondition.label;
                 }
@@ -201 +200 @@
         }
         return key;
-    }
-
-    private static boolean conditionRequiresKeyPresence(KeyMatchType matchType) {
-        return matchType != KeyMatchType.REGEX;
     }
 

trunk/test/unit/org/openstreetmap/josm/gui/mappaint/mapcss/KeyConditionTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.mappaint.mapcss;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -12 +13 @@
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.OsmUtils;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -82 +84 @@
         ConditionFactory.createKeyCondition("a key", true, KeyMatchType.TRUE, Context.PRIMITIVE);
 
+        // [/regex/]
+        Condition c = ConditionFactory.createKeyCondition("foo|bar", false, KeyMatchType.REGEX, Context.PRIMITIVE);
+        assertTrue(c.applies(new Environment(OsmUtils.createPrimitive("node BARfooBAZ=true"))));
+        assertFalse(c.applies(new Environment(OsmUtils.createPrimitive("node BARBAZ=true"))));
+        c = ConditionFactory.createKeyCondition("colour:", false, KeyMatchType.REGEX, Context.PRIMITIVE);
+        assertEquals(KeyMatchType.ANY_CONTAINS, ((KeyCondition) c).matchType);
+        assertEquals("colour:", ((KeyCondition) c).label);
+        assertTrue(c.applies(new Environment(OsmUtils.createPrimitive("node colour:roof=ref"))));
+        assertFalse(c.applies(new Environment(OsmUtils.createPrimitive("node foo=bar"))));
+        c = ConditionFactory.createKeyCondition("^wikipedia:", false, KeyMatchType.REGEX, Context.PRIMITIVE);
+        assertEquals(KeyMatchType.ANY_STARTS_WITH, ((KeyCondition) c).matchType);
+        assertEquals("wikipedia:", ((KeyCondition) c).label);
+        assertTrue(c.applies(new Environment(OsmUtils.createPrimitive("node wikipedia:en=a"))));
+        assertFalse(c.applies(new Environment(OsmUtils.createPrimitive("node wikipedia=a"))));
+        c = ConditionFactory.createKeyCondition("_name$", false, KeyMatchType.REGEX, Context.PRIMITIVE);
+        assertEquals(KeyMatchType.ANY_ENDS_WITH, ((KeyCondition) c).matchType);
+        assertEquals("_name", ((KeyCondition) c).label);
+        assertTrue(c.applies(new Environment(OsmUtils.createPrimitive("node alt_name=a"))));
+        assertFalse(c.applies(new Environment(OsmUtils.createPrimitive("node name=a"))));
+
         // ["a label"]
         ConditionFactory.createKeyCondition("a key", false, null, Context.LINK);