Modify

Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#18964 closed enhancement (fixed)

[Patch] MapCSS rules using index are slow

Reported by: GerdP Owned by: team
Priority: minor Milestone: 20.03
Component: Core validator Version:
Keywords: validator mapcss performance Cc:

Description

For the validator we have several rules like this:

way >[index=1] node,
way >[index=-1] node {
  set first_last_node;
}

The current implementation for the index test works like this:
Perform a sequential search for the given node in the parent way. If found, compare the position with the value given with index.
I think it should work the other way around: Check if the node at the given position exists and if so compare it with the given node.

Attachments (1)

18964.patch (2.4 KB) - added by GerdP 2 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 months ago by GerdP

Priority: normalminor
Resolution: wontfix
Status: newclosed

Probably not a problem with real OSM data. I've noticed this with a generated file containing coastline data for Great Britain. The data has several ways with 64000 nodes. Only with this extreme case the sequential search is a problem.

For normal data the additional checks to avoid this search are even more complex.

Changed 2 months ago by GerdP

Attachment: 18964.patch added

comment:2 Changed 2 months ago by GerdP

Resolution: wontfix
Status: closedreopened
Summary: MapCSS rules using index are slow[Patch] MapCSS rules using index are slow

The attached patch improves performance drastically for the mentioned special cases, saw no changes with average data.
is it worth the additional code?

comment:3 Changed 2 months ago by Don-vip

I'd say yes. Just make the firstAndLastOnly a private method so that the calling code simplifies to:

private boolean firstAndLastOnly() {
    for (Condition c : link.conds) {
        if (!(c instanceof IndexCondition) || !((IndexCondition) c).isFirstOrLast) {
            return false;
        }
    }
    return true;
}

/// ...

int step = firstAndLastOnly() ? count - 1 : 1;

comment:4 Changed 2 months ago by GerdP

Resolution: fixed
Status: reopenedclosed

In 16199/josm:

fix #18964: MapCSS rules using index are slow
Avoid a sequential search through all nodes when index matches first or last node only

comment:5 Changed 2 months ago by GerdP

Milestone: 20.03

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.