Changeset 18823 in josm for trunk


Ignore:
Timestamp:
2023-09-06T22:04:23+02:00 (10 months ago)
Author:
taylor.smock
Message:

Improve performance for TaggingPresetSelector$PresetClassifications.getMatchingPresets

This was found when profiling with the Name Suggestion Index enabled.

The improvements are as follows:

  • ~80% reduction in CPU usage
  • ~85% reduction in memory allocations

A good chunk of the remaining cpu cycles and memory allocations is from the sort
operation.

Non-performance related changes are as follows:

  • Add documentation
  • Lint fixes
Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSelector.java

    r18691 r18823  
    99import java.awt.event.ActionEvent;
    1010import java.util.ArrayList;
     11import java.util.Arrays;
    1112import java.util.Collection;
    1213import java.util.Collections;
    … …  
    6465
    6566    private static final Pattern PATTERN_PUNCTUATION = Pattern.compile("\\p{Punct}", Pattern.UNICODE_CHARACTER_CLASS);
     67    private static final Pattern PATTERN_WHITESPACE = Pattern.compile("\\s", Pattern.UNICODE_CHARACTER_CLASS);
    6668
    6769    private static final BooleanProperty SEARCH_IN_TAGS = new BooleanProperty("taggingpreset.dialog.search-in-tags", true);
    … …  
    9092     */
    9193    public static class PresetClassification implements Comparable<PresetClassification> {
     94        /** The preset for this classification object */
    9295        public final TaggingPreset preset;
     96        /**
     97         * The classification for the preset (see {@link #CLASSIFICATION_TAGS_MATCH}, {@link #CLASSIFICATION_GROUP_MATCH},
     98         * {@link #CLASSIFICATION_NAME_MATCH}, and {@link #CLASSIFICATION_IN_FAVORITES}). Higher numbers are better.
     99         */
    93100        public int classification;
     101        /**
     102         * The index in favorites, index = {@link #CLASSIFICATION_IN_FAVORITES} - favoriteIndex
     103         */
    94104        public int favoriteIndex;
    95         private final Collection<String> groups;
    96         private final Collection<String> names;
    97         private final Collection<String> tags;
     105        /** Groups that have been run through {@link #simplifyString(String)} */
     106        private final String[] groupsSimplified;
     107        /** Names that have been run through {@link #simplifyString(String)}*/
     108        private final String[] namesSimplified;
     109        /** Tags that have been run through {@link #simplifyString(String)} */
     110        private final String[] tagsSimplified;
    98111
    99112        PresetClassification(TaggingPreset preset) {
    … …  
    126139                }
    127140            }
    128             this.groups = Utils.toUnmodifiableList(groupSet);
    129             this.names = Utils.toUnmodifiableList(nameSet);
    130             this.tags = Utils.toUnmodifiableList(tagSet);
     141            // These should be "frozen" arrays
     142            this.groupsSimplified = groupSet.stream().map(PresetClassification::simplifyString)
     143                    .toArray(String[]::new);
     144            this.namesSimplified = nameSet.stream().map(PresetClassification::simplifyString)
     145                    .toArray(String[]::new);
     146            this.tagsSimplified = tagSet.stream().map(PresetClassification::simplifyString)
     147                    .toArray(String[]::new);
    131148        }
    132149
    … …  
    134151            String locName = preset.getLocaleName();
    135152            if (locName != null) {
    136                 Collections.addAll(collection, locName.toLowerCase(Locale.ENGLISH).split("\\s", -1));
     153                Collections.addAll(collection, PATTERN_WHITESPACE.split(locName.toLowerCase(Locale.ENGLISH), -1));
    137154            }
    138155        }
    … …  
    142159        }
    143160
    144         private static int isMatching(Collection<String> values, String... searchString) {
     161        /**
     162         * Check to see if the search string matches values
     163         * @param deaccentedValues Values that have been simplified
     164         * @param deaccentedSearchString The simplified search string to use
     165         * @return The number used for sorting hits (bigger == more matches)
     166         */
     167        private static int isMatching(String[] deaccentedValues, String... deaccentedSearchString) {
    145168            int sum = 0;
    146             List<String> deaccentedValues = values.stream()
    147                     .map(PresetClassification::simplifyString).collect(Collectors.toList());
    148             for (String word: searchString) {
     169            for (String deaccentedWord: deaccentedSearchString) {
    149170                boolean found = false;
    150171                boolean foundFirst = false;
    151                 String deaccentedWord = simplifyString(word);
    152172                for (String value: deaccentedValues) {
    153173                    int index = value.indexOf(deaccentedWord);
    … …  
    169189        }
    170190
    171         int isMatchingGroup(String... words) {
    172             return isMatching(groups, words);
    173         }
    174 
    175         int isMatchingName(String... words) {
    176             return isMatching(names, words);
    177         }
    178 
    179         int isMatchingTags(String... words) {
    180             return isMatching(tags, words);
     191        private int isMatchingGroup(String... words) {
     192            return isMatching(groupsSimplified, words);
     193        }
     194
     195        private int isMatchingName(String... words) {
     196            return isMatching(namesSimplified, words);
     197        }
     198
     199        private int isMatchingTags(String... words) {
     200            return isMatching(tagsSimplified, words);
    181201        }
    182202
    … …  
    188208            else
    189209                return result;
     210        }
     211
     212        @Override
     213        public int hashCode() {
     214            return this.preset.hashCode() + 31 * (Integer.hashCode(this.classification)
     215                    + 31 * Integer.hashCode(this.favoriteIndex));
     216        }
     217
     218        @Override
     219        public boolean equals(Object obj) {
     220            if (this.getClass().isInstance(obj)) {
     221                PresetClassification other = (PresetClassification) obj;
     222                return this.preset.equals(other.preset) && this.classification == other.classification
     223                        && this.favoriteIndex == other.favoriteIndex;
     224            }
     225            return false;
    190226        }
    191227
    … …  
    257293
    258294        DataSet ds = OsmDataManager.getInstance().getEditDataSet();
    259         Collection<OsmPrimitive> selected = (ds == null) ? Collections.<OsmPrimitive>emptyList() : ds.getSelected();
     295        Collection<OsmPrimitive> selected = (ds == null) ? Collections.emptyList() : ds.getSelected();
    260296        final List<PresetClassification> result = classifications.getMatchingPresets(
    261297                text, onlyApplicable, inTags, getTypesInSelection(), selected);
    … …  
    277313     */
    278314    public static class PresetClassifications implements Iterable<PresetClassification> {
    279 
    280         private final List<PresetClassification> classifications = new ArrayList<>();
    281 
     315        private static final PresetClassification[] EMPTY_PRESET_CLASSIFICATION = new PresetClassification[0];
     316        private PresetClassification[] classifications = EMPTY_PRESET_CLASSIFICATION;
     317
     318        /**
     319         * Get matching presets
     320         * @param searchText The text to search for
     321         * @param onlyApplicable Only look for presets that are applicable to the selection
     322         * @param inTags Search for names in tags
     323         * @param presetTypes The preset types to look for, may be {@code null}
     324         * @param selectedPrimitives The primitives to filter on, must not be {@code null}
     325         *                           if {@code onlyApplicable} is {@code true}
     326         * @return The matching presets in a sorted list based off of relevance.
     327         */
    282328        public List<PresetClassification> getMatchingPresets(String searchText, boolean onlyApplicable, boolean inTags,
    283329                Set<TaggingPresetType> presetTypes, final Collection<? extends OsmPrimitive> selectedPrimitives) {
    … …  
    286332
    287333            if (searchText.contains("/")) {
    288                 groupWords = searchText.substring(0, searchText.lastIndexOf('/')).split("[\\s/]", -1);
    289                 nameWords = searchText.substring(searchText.indexOf('/') + 1).split("\\s", -1);
     334                groupWords = searchText.substring(0, searchText.lastIndexOf('/')).split("(?U)[\\s/]", -1);
     335                nameWords = PATTERN_WHITESPACE.split(searchText.substring(searchText.indexOf('/') + 1), -1);
    290336            } else {
    291337                groupWords = null;
    292                 nameWords = searchText.split("\\s", -1);
     338                nameWords = PATTERN_WHITESPACE.split(searchText, -1);
    293339            }
    294340
    … …  
    296342        }
    297343
     344        /**
     345         * Get matching presets
     346         * @param groupWords The groups to search for
     347         * @param nameWords The names to search for, may look in tags if {@code inTags} is {@code true}
     348         * @param onlyApplicable Only look for presets that are applicable to the selection
     349         * @param inTags Search for names in tags
     350         * @param presetTypes The preset types to look for, may be {@code null}
     351         * @param selectedPrimitives The primitives to filter on, must not be {@code null}
     352         *                           if {@code onlyApplicable} is {@code true}
     353         * @return The matching presets in a sorted list based off of relevance.
     354         */
    298355        public List<PresetClassification> getMatchingPresets(String[] groupWords, String[] nameWords, boolean onlyApplicable,
    299356                boolean inTags, Set<TaggingPresetType> presetTypes, final Collection<? extends OsmPrimitive> selectedPrimitives) {
    300357
    301358            final List<PresetClassification> result = new ArrayList<>();
     359            final String[] simplifiedGroupWords = groupWords == null ? null :
     360                    Arrays.stream(groupWords).map(PresetClassification::simplifyString).toArray(String[]::new);
     361            final String[] simplifiedNameWords = nameWords == null ? null :
     362                    Arrays.stream(nameWords).map(PresetClassification::simplifyString).toArray(String[]::new);
    302363            for (PresetClassification presetClassification : classifications) {
    303364                TaggingPreset preset = presetClassification.preset;
    … …  
    318379                }
    319380
    320                 if (groupWords != null && presetClassification.isMatchingGroup(groupWords) == 0) {
     381                if (simplifiedGroupWords != null && presetClassification.isMatchingGroup(simplifiedGroupWords) == 0) {
    321382                    continue;
    322383                }
    323384
    324                 int matchName = presetClassification.isMatchingName(nameWords);
     385                int matchName = presetClassification.isMatchingName(simplifiedNameWords);
    325386
    326387                if (matchName == 0) {
    327                     if (groupWords == null) {
    328                         int groupMatch = presetClassification.isMatchingGroup(nameWords);
     388                    if (simplifiedGroupWords == null) {
     389                        int groupMatch = presetClassification.isMatchingGroup(simplifiedNameWords);
    329390                        if (groupMatch > 0) {
    330391                            presetClassification.classification = CLASSIFICATION_GROUP_MATCH + groupMatch;
    … …  
    332393                    }
    333394                    if (presetClassification.classification == 0 && inTags) {
    334                         int tagsMatch = presetClassification.isMatchingTags(nameWords);
     395                        int tagsMatch = presetClassification.isMatchingTags(simplifiedNameWords);
    335396                        if (tagsMatch > 0) {
    336397                            presetClassification.classification = CLASSIFICATION_TAGS_MATCH + tagsMatch;
    … …  
    355416         */
    356417        public void clear() {
    357             classifications.clear();
     418            classifications = EMPTY_PRESET_CLASSIFICATION;
    358419        }
    359420
    … …  
    363424         */
    364425        public void loadPresets(Collection<TaggingPreset> presets) {
     426            final List<PresetClassification> classificationList = new ArrayList<>(presets.size());
    365427            for (TaggingPreset preset : presets) {
    366428                if (preset instanceof TaggingPresetSeparator || preset instanceof TaggingPresetMenu) {
    367429                    continue;
    368430                }
    369                 classifications.add(new PresetClassification(preset));
    370             }
     431                classificationList.add(new PresetClassification(preset));
     432            }
     433            classifications = classificationList.toArray(new PresetClassification[0]);
    371434        }
    372435
    373436        @Override
    374437        public Iterator<PresetClassification> iterator() {
    375             return classifications.iterator();
     438            return Arrays.stream(classifications).iterator();
    376439        }
    377440    }

  • trunk/test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSelectorTest.java

    r17275 r18823  
    22package org.openstreetmap.josm.gui.tagging.presets;
    33
    4 import static org.junit.jupiter.api.Assertions.assertEquals;
     4import static org.junit.jupiter.api.Assertions.assertSame;
    55import static org.junit.jupiter.api.Assertions.assertTrue;
    66
    7 import org.junit.jupiter.api.extension.RegisterExtension;
     7import java.util.Collections;
     8
    89import org.junit.jupiter.api.Test;
    9 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetSelector.PresetClassification;
    10 import org.openstreetmap.josm.testutils.JOSMTestRules;
    11 
    12 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
    1310
    1411/**
    … …  
    1613 */
    1714class TaggingPresetSelectorTest {
    18 
    1915    /**
    20      * Setup rule
    21      */
    22     @RegisterExtension
    23     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
    24     public JOSMTestRules test = new JOSMTestRules();
    25 
    26     /**
    27      * Unit test for {@link PresetClassification#isMatching}.
     16     * Unit test for {@link TaggingPresetSelector.PresetClassifications#getMatchingPresets}.
    2817     */
    2918    @Test
    30     void testIsMatching() {
     19    void testGetMatching() {
    3120        TaggingPreset preset = new TaggingPreset();
    3221        preset.name = "estação de bombeiros"; // fire_station in brazilian portuguese
    33         PresetClassification pc = new PresetClassification(preset);
    34         assertEquals(0, pc.isMatchingName("foo"));
    35         assertTrue(pc.isMatchingName("estação") > 0);
    36         assertTrue(pc.isMatchingName("estacao") > 0);
     22        TaggingPresetSelector.PresetClassifications presetClassifications = new TaggingPresetSelector.PresetClassifications();
     23        presetClassifications.loadPresets(Collections.singleton(preset));
     24        assertTrue(presetClassifications.getMatchingPresets(null, new String[] {"foo"}, false,
     25                false, null, null).isEmpty());
     26        assertSame(preset, presetClassifications.getMatchingPresets(null, new String[] {"estação"}, false,
     27                false, null, null).get(0).preset);
     28        assertSame(preset, presetClassifications.getMatchingPresets(null, new String[] {"estacao"}, false,
     29                false, null, null).get(0).preset);
    3730    }
    3831}
Note: See TracChangeset for help on using the changeset viewer.