Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#11834 closed enhancement (fixed)

[Patch] Use visitor pattern for key/value lookup.

Reported by: michael2402 Owned by: team
Priority: normal Milestone: 15.09
Component: Core mappaint Version:
Keywords: Cc:

Description

Currently, MapCSS creates a HashMap to get the keys from each primitive.

We can speed this up a lot using a visitor pattern.

Attachments (1)

0001-Made-MapCSS-use-the-visitor-pattern-to-retrive-tags-.patch (4.8 KB) - added by michael2402 5 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 5 years ago by simon04

Milestone: 15.09

Looks good to me. Seems to be a safer alternative to #11653.

comment:2 Changed 5 years ago by simon04

Resolution: fixed
Status: newclosed

In 8740/josm:

fix #11834 - Use visitor pattern for key/value lookup (patch by michael2402, slightly modified)

comment:3 Changed 5 years ago by simon04

Resolution: fixed
Status: closedreopened

While attempting to use this visitor for the UntaggedNode test, I needed also to obtain the current primitive. The following modification also provides the primitive to the visitor (which can be ignored when not needed). What do you think?

  • src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java

    diff --git a/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java b/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java
    index 490764f..991df5e 100644
    a b public abstract class AbstractPrimitive implements IPrimitive { 
    3737        /**
    3838         * This method gets called for every tag received.
    3939         *
     40         * @param primitive This primitive
    4041         * @param key   The key
    4142         * @param value The value
    4243         */
    43         void visitKeyValue(String key, String value);
     44        void visitKeyValue(AbstractPrimitive primitive, String key, String value);
    4445    }
    4546
    4647    private static final AtomicLong idCounter = new AtomicLong(0);
    public abstract class AbstractPrimitive implements IPrimitive { 
    519520        final String[] keys = this.keys;
    520521        if (keys != null) {
    521522            for (int i = 0; i < keys.length; i += 2) {
    522                 visitor.visitKeyValue(keys[i], keys[i + 1]);
     523                visitor.visitKeyValue(this, keys[i], keys[i + 1]);
    523524            }
    524525        }
    525526    }

comment:4 Changed 5 years ago by michael2402

This is a good improvement. The overhead should be minimal (since we are only passing one more parameter).

Don't you rather want to use a visitor for each node in that test? You could let it collect all information for the node and then display the result. Performance should not be that critical there.

comment:5 Changed 5 years ago by simon04

Resolution: fixed
Status: reopenedclosed

In 8742/josm:

fix #11834 - KeyValueVisitor: also provide primitive to the visitor

comment:6 Changed 5 years ago by simon04

In 8743/josm:

see #11834 - Use KeyValueVisitor in the UntaggedNode test

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.