Possible performance problem in MapCSS based Tag checker

[GerdP]

What steps will reproduce the problem?

See #16976

What is the expected result?

No debug message

What happens instead?

Two messeages are printed.

Please provide any additional information below. Attach a screenshot if possible.

It seems that these rules in addresses.mapcss are matched although there is no way in the dataset:

way[addr:interpolation=odd] > node[addr:housenumber][get(split(".", tag("addr:housenumber")/2), 1)=0] {
    throwWarning: tr("Even housenumber in odd address interpolation.");
}
way[addr:interpolation=even] > node[addr:housenumber][get(split(".", tag("addr:housenumber")/2), 1)=5] {
    throwWarning: tr("Odd housenumber in even address interpolation.");
}

It seems that the selector works in the wrong way?

Build-Date:2018-11-10 15:44:56
Revision:14419
Is-Local-Build:true

Identification: JOSM/1.5 (14419 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1803 (17134)
Memory Usage: 563 MB / 3641 MB (230 MB allocated, but free)
Java version: 1.8.0_191-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-agentlib:jdwp=transport=dt_socket,suspend=y,address=localhost:54845, -ea, -Dfile.encoding=UTF-8]
Program arguments: [--debug]

Plugins:
+ OpeningHoursEditor (34535)
+ apache-commons (34506)
+ buildings_tools (34572)
+ download_along (34503)
+ ejml (34389)
+ geotools (34513)
+ jaxb (34506)
+ jts (34524)
+ measurement (34529)
+ merge-overlap (34664)
+ o5m (34405)
+ opendata (34698)
+ pbf (34576)
+ poly (34546)
+ reverter (34552)
+ undelete (34568)
+ utilsplugin2 (34506)

Last errors/warnings:
- W: Update plugins - org.openstreetmap.josm.plugins.PluginHandler$UpdatePluginsMessagePanel[,0,0,0x0,invalid,layout=java.awt.GridBagLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=]
- W: No configuration settings found.  Using hardcoded default values for all pools.
- W: Failed to locate resource '/README'.
- W: Failed to locate resource '/CONTRIBUTION'.
- W: Failed to locate resource '/LICENSE'.

[GerdP]

The rules seem to say something like "if you test a way and the way has the tag addr:interpolation=odd then check the nodes of the way to have odd housenumbers.
Presuming you have a normal dataset with some rels, more ways and many nodes this is what happens:
For each node the housenumber is extracted and checked if it is even. If that is the case, the parent is checked if it is a way with tag addr:interpolation=odd.
Since we have much more nodes than ways we perform the test far more often than necessary, besides that the check is more complex than the rather simple test for the tag addr:interpolation=odd.
I don't know if it is possible to rewrite those rules or if it would make sense to rewrite the complex code in Selector?

[GerdP]

It seems something is wrong in the mapcss parser. I'd expect that a rule like

way[highway =~ /^(bridleway|cycleway|footway|path|steps)$/] > node { set .is_in_minor_road }

would be processed by a ChildOrParentSelector of type PARENT, instead the type is CHILD.
It seems that type PARENT is never used. When you look at the generated code in MapCSSParser.java you can see that the switch case
statement staring in line 942 will never reach line 968 because

case LESS:

is processed before.

[GerdP]

Forget #comment:3
The rule has to be

node < way[highway =~ /^(bridleway|cycleway|footway|path|steps)$/] { set .is_in_minor_road }

when a PARENT selector is wanted. If I got that right, this selector is a specialty in JOSM. Is that the reason why it is not used
anywhere in the mapcss rules?

[GerdP]

No, the problem is that the rewritten rule will only work for the first node of the way. Same problem when I rewrite

way[addr:interpolation=odd] > node[addr:housenumber][get(split(".", tag("addr:housenumber")/2), 1)=0] {
    throwWarning: tr("Even housenumber in odd address interpolation.");
}

to

node[addr:housenumber][get(split(".", tag("addr:housenumber")/2), 1)=0] < way[addr:interpolation=odd] {
    throwWarning: tr("Even housenumber in odd address interpolation.");
}

In a way with two odd and two even numbers only one wrong node would be flagged, not both, because the selector stops at the first match.
So, type PARENT is probably not useful for validator rules, maybe not at all?
So, still no idea how to avoid all those useless complex checks on nodes :(

[GerdP]

A change would probably require a lot more memory and might not work at all.

[skyper]

How about:

way[addr:interpolation =~ /^(even|odd)$/] > node ( set ChildAddrInterpol;}

node[addr:housenumber]get(split(".", tag("addr:housenumber")/2), 1)=0].ChildAddrInterpol {
    throwWarning: tr("Even housenumber in odd address interpolation.");
}
node[addr:housenumber][get(split(".", tag("addr:housenumber")/2), 1)=5].ChildAddrInterpol {
    throwWarning: tr("Odd housenumber in even address interpolation.");
}

Does the order of selectors matter? From which side is evaluated?

Maybe even below will work, which will only look for parent tag:

node[addr:housenumber][parent_tag("addr:interpolation") == odd][get(split(".", tag("addr:housenumber")/2), 1)=0] {
    throwWarning: tr("Even housenumber in odd address interpolation.");
}
node[addr:housenumber][parent_tag("addr:interpolation") == even][get(split(".", tag("addr:housenumber")/2), 1)=5] {
    throwWarning: tr("Odd housenumber in even address interpolation.");
}

[GerdP]

No idea if those rules improve performance. I wasn't so much interested in those specific rules when I opened the ticket.
What bothered me was that the node selector criteria were evaluated even though the way selector could not match and I (wrongly) assumed that the evaluation would start with the way. Since we want one warning for each node we have to test the node, not the way. Else we could only produce a warning like "Even address interpolation has node with odd number" without being able to hilite the odd node.