Hi Steve, thanks for review, I've added a unit test to show why your version doesn't work. I see no simple way to improve the index much more. I thought about a different approach: Instead of doing all the calculations before any rule is really executed we might do this dynamically. Each rule which changes or adds tags would point to a list of further rules which are to be checked. Only when such a change really happens the additional rules are checked. No idea if this can be implemented more efficiently than the current static index. Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Steve Ratcliffe <steve@parabola.me.uk> Gesendet: Dienstag, 24. April 2018 23:06:11 An: mkgmap-dev@lists.mkgmap.org.uk Betreff: Re: [mkgmap-dev] patch to improve style throughput Hi Gerd I don't think I understand any of this any more :( It didn't seem right to extract the key from the keystring so I tried without and the attached patch also passes all the tests - I make no other claim! Steve

Hi Steve,

I think I found a simple patch to improve the rule index. I've noticed that the unpatched version often returns far more rules to check than expected. e.g. if an element has the tag a=3 and we have these 3 rules a=1 {...} a=2 & b=1 {...} a=3 & c=1 {...}

all three are evaluated, in fact all rules which have a keystring beginning with a= are returned by the index. The patch changes this so that rules which cannot match are not returned. This makes style evaluation a bit faster. I've tried it with a few styles and some input files and found no changes in the output, also all unit tests pass, so I hope I did not miss something. What do you think?

Gerd

_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Hi Steve, yes, would be good to separate those cases. I'll see if I can implement that. Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Steve Ratcliffe <steve@parabola.me.uk> Gesendet: Mittwoch, 25. April 2018 23:28:45 An: mkgmap-dev@lists.mkgmap.org.uk Betreff: Re: [mkgmap-dev] patch to improve style throughput Hi Gerd,

thanks for review, I've added a unit test to show why your version doesn't work.

Yes, that doesn't work. Well, I'll just say what I was trying to do. With these rules: a=* {set b=2} b=1 [0x10404 resolution 24] and an element with a=1, it is not possible to match the b=1 rule, but it is still tried anyway. In this case changeableTags contains the full b=2 keystring so it ought to be possible to recognise that the second rule cannot match. ..Steve

I see no simple way to improve the index much more. I thought about a different approach: Instead of doing all the calculations before any rule is really executed we might do this dynamically. Each rule which changes or adds tags would point to a list of further rules which are to be checked. Only when such a change really happens the additional rules are checked. No idea if this can be implemented more efficiently than the current static index.

_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Hi Gerd

here is the improved version of the patch. It reduces the number of rules to be checked a bit more.

Thanks, that seems to work well. Steve

Hi all, I think I found a much better solution for the RuleIndex. Attached patch reduces again the number of evaluated rules, but should still evaluates all necessary rules. The old index still returned too many rules. Let's look at the example that I added as unit test. Set of rules: 0: a=* {set b=1} 1: b=1 {set c=1} 2: d=2 {set c=2} 3: c=* {set a=2} 4: c=1 {set d=2} 5: c=2 {set d=1} 6: d=1 [0x10401 resolution 24] 7: d=2 [0x10402 resolution 24] Assume we have no index and an element with only one tag a=1. We check each rule: Rules 0 matches and sets b=1 Now 1 matches and sets c=1 Rule 2 doesn't match. Rule 3 matches and sets a=2 Rule 4 matches and sets d=2 Rule 5 doesn't match. Rule 6 doesn't match. Rule 7 matches. The index is used to reduce the number of evaluated rules. In the best case it should return rules 0,1,3,4, and 7 for the given element. But the current code returns all rules because it thinks that rule 4 can set d=2 and therefore rule 2 might be triggered (which is wrong). The patch changes the index to recognizes the order of the rules which change tags and therefore returns only the needed rules. I've tested it with different styles and found no problem so far. Means, maps did not change and the patch always was better than trunk or tags4.patch. A binary based on trunk r4183 is here: http://files.mkgmap.org.uk/download/432/mkgmap.jar @Steve: Please let me know if you can think of a case that might not work. If I hear no complains I'll commit this patch next weekend. Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Gerd Petermann <gpetermann_muenchen@hotmail.com> Gesendet: Samstag, 28. April 2018 09:34:36 An: Development list for mkgmap Betreff: Re: [mkgmap-dev] patch to improve style throughput Hi Steve, after committing the tags3.patch with r4182 yesterday I wondered why I did not find a unit test which shows the problem that I fixed with r3822: http://www.mkgmap.org.uk/websvn/revision.php?repname=mkgmap&rev=3822 I've now added that test to make sure that the new code works. It does :) I got curious and tried RuleIndex.java from r3820 and was surprised to find that it alse passes all tests. In the end the code in RuleIndex was okay, the error was in the code that arranged expressions and you fixed that recently. The good thing is that the old code was even better than that in tags3.patch, so I've created a new patch which is basically a revert of the changes in r3822 and r4182. Please review and try to find a unit test that would not work with that patch. Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Steve Ratcliffe <steve@parabola.me.uk> Gesendet: Donnerstag, 26. April 2018 23:33:53 An: mkgmap-dev@lists.mkgmap.org.uk Betreff: Re: [mkgmap-dev] patch to improve style throughput Hi Gerd

here is the improved version of the patch. It reduces the number of rules to be checked a bit more.

Thanks, that seems to work well. Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Hi Steve, thanks for testing. I think your idea would require more work with little gain: 1) We have to make sure that no action deletes a. This information is not available now. 2) We have to know much more info about the rules than what we get in keystring. 3) We have to change the way how we access the index. We only gain a smaller set of rules but we would still evaluate the same amount of rules. Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Steve Ratcliffe <steve@parabola.me.uk> Gesendet: Dienstag, 1. Mai 2018 23:49:01 An: mkgmap-dev@lists.mkgmap.org.uk Betreff: Re: [mkgmap-dev] patch to improve style throughput Hi Gerd,

@Steve: Please let me know if you can think of a case that might not work.

It looks good, I couldn't find anything that didn't work. It may not be possible or worth doing this, but if a rule can resolve, then its actions can be ignored unless continue with_actions/propagate is set. So in the following, rules 2 and 4 can never match with a=1. 0: a=* {set b='${something}'} 1: b=2 {set c=2} [0x1] 2: c=* {set a=2} 3: c=1 {set d=2} 4: c=2 {set d=1} 5: d=1 [0x10401 resolution 24] 6: d=2 [0x10402 resolution 24] 7: b=1 [0x2] Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev